Skip to content

Adds test scenarios for Basic02#1380

Merged
matsduf merged 10 commits into
zonemaster:developfrom
matsduf:add-basic02-scenarios-and-test-zones
May 20, 2025
Merged

Adds test scenarios for Basic02#1380
matsduf merged 10 commits into
zonemaster:developfrom
matsduf:add-basic02-scenarios-and-test-zones

Conversation

@matsduf

@matsduf matsduf commented Apr 11, 2025

Copy link
Copy Markdown
Contributor

Purpose

This PR adds test scenarios for Basic02.

The way IP addresses are loaded have changed to be much faster.

Context

These scenarios will be implemented as unit tests in Engine, see zonemaster/zonemaster-engine#1447

How to test this PR

  • Review.
  • When implemented as unit tests, they should pass.

Comment thread docs/public/specifications/test-zones/Basic-TP/basic02.md Outdated
Comment thread docs/public/specifications/test-zones/Basic-TP/basic02.md Outdated
Comment thread docs/public/specifications/test-zones/Basic-TP/basic02.md Outdated
Comment thread docs/public/specifications/test-zones/Basic-TP/basic02.md
Comment thread docs/public/specifications/test-zones/Basic-TP/basic02.md Outdated
Comment thread docs/public/specifications/test-zones/Basic-TP/basic02.md Outdated
Comment thread docs/public/specifications/test-zones/Basic-TP/basic02.md Outdated
Comment thread docs/public/specifications/test-zones/Basic-TP/basic02.md Outdated
Comment thread docs/public/specifications/test-zones/Basic-TP/basic02.md Outdated
Comment thread docs/public/specifications/test-zones/Basic-TP/basic02.md Outdated
matsduf and others added 3 commits April 25, 2025 15:00
* Makes set-ip.sh run much faster. Required now to be run by explicit
  sudo.
* Requires start-coredns.sh to be run by explicit sudo to be consistent.
* Updates documentation in README.md.
Co-authored-by: Marc van der Wal <103426270+marc-vanderwal@users.noreply.github.com>
@matsduf matsduf requested a review from marc-vanderwal April 25, 2025 15:49
Comment thread docs/public/specifications/test-zones/Basic-TP/basic02.md Outdated
Comment thread test-zone-data/Basic-TP/basic02/good-undel-7.basic02.xa.zone Outdated
Comment on lines +194 to +195
* ns1.good-undel-4.basic02.xb
* ns2.good-undel-4.basic02.xb

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think?

Suggested change
* ns1.good-undel-4.basic02.xb
* ns2.good-undel-4.basic02.xb
* ns3.good-undel-4.basic02.xb
* ns4.good-undel-4.basic02.xb

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. Why do you think so?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are in a different TLD, xb instead of xa. It’s easy to miss without very careful reading, though.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is nothing incorrect in the naming. It is stated that the names are OOB, which means that reading as ".xa" gives an impossible name. Another name is possible (e.g. "dns1" and "dns2") but a change requires update in several files and also in another PR.

Is it requested that I do that?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compare with the previous scenario (GOOD-UNDEL-3) which should be roughly similar in setup. And see file test-zone-data/Basic-TP/basic02/good-undel-4.basic02.xa.zone where ns{3, 4}.good-undel-4.basic02.xb. are listed as zone NS.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To keep down confusion I use 127.12.2.31 for ns1 and 127.12.2.33 for ns3.

I do not see why ns3/ns4 makes anything clearer. You have to check the configuration for each scenario. Such a change will require in several places in two PRs.

I will correct the zone file.

Comment thread docs/public/specifications/test-zones/Basic-TP/basic02.md Outdated
Comment thread docs/public/specifications/test-zones/Basic-TP/basic02.md Outdated
Comment thread docs/public/specifications/test-zones/Basic-TP/basic02.md Outdated
Comment thread docs/public/specifications/test-zones/Basic-TP/basic02.md Outdated
Comment thread docs/public/specifications/test-zones/Basic-TP/basic02.md Outdated
Co-authored-by: tgreenx <96772376+tgreenx@users.noreply.github.com>
@matsduf matsduf requested a review from tgreenx May 1, 2025 15:21
@matsduf

matsduf commented May 1, 2025

Copy link
Copy Markdown
Contributor Author

@tgreenx and @marc-vanderwal, please rereview.

marc-vanderwal
marc-vanderwal previously approved these changes May 7, 2025
Comment on lines +194 to +195
* ns1.good-undel-4.basic02.xb
* ns2.good-undel-4.basic02.xb

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compare with the previous scenario (GOOD-UNDEL-3) which should be roughly similar in setup. And see file test-zone-data/Basic-TP/basic02/good-undel-4.basic02.xa.zone where ns{3, 4}.good-undel-4.basic02.xb. are listed as zone NS.

Comment thread test-zone-data/Basic-TP/basic02/basic02.cfg
@matsduf

matsduf commented May 20, 2025

Copy link
Copy Markdown
Contributor Author

@tgreenx and @marc-vanderwal, you have approved zonemaster/zonemaster-engine#1447. Can this PR also be approved?

@matsduf matsduf merged commit ce0f926 into zonemaster:develop May 20, 2025
@matsduf matsduf deleted the add-basic02-scenarios-and-test-zones branch May 20, 2025 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-TestCase Area: Test case specification or implementation of test case

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants