Fix Consistency06 test scenario MULT-SOA-MNAMES-NO-DEL-UNDEL-2#1414
Merged
marc-vanderwal merged 3 commits intoOct 8, 2025
Merged
Conversation
marc-vanderwal
added a commit
to marc-vanderwal/zonemaster-engine
that referenced
this pull request
Jul 28, 2025
Another rewrite of a unit test that led me to uncover a typo in the original unit test. Actually, this time, the specification was incorrect (see zonemaster/zonemaster#1414).
Contributor
|
The specification says ns1 and ns2, seems to use ns3 and ns4. The specification needs an update. |
Contributor
Author
|
Yes indeed, there is another problem in that scenario specification. The first bullet point says “NS are out-of-bailiwick, "ns3.mult-soa-mnames-no-del-undel-2.consistency06.xb" and "ns4.mult-soa-mnames-no-del-undel-2.consistency06.xb"”. But the next bullet points talk about “ns1” and “ns2”. All we really need is for ns3 and ns4 to return SOA records that differ in their MNAME fields. |
In the specification for test scenarios for Consistency06, scenario MULT-SOA-MNAMES-NO-DEL-UNDEL-2 erroneously lists ns3.mult-soa-mnames-no-del-undel-2.consistency06.xb twice in the undelegated data. The second occurrence should be ns4, not ns3. Right above, the description of what SOA MNAME field values appear mention name servers named ns1 and ns2, where ns3 and ns4 was probably meant. Unfortunately for us, fixing the first discrepancy does not fix the underlying problem: it instead underscores that there is a bug in the current implementation of Consistency06.
5b0d94b to
aef6a7d
Compare
Contributor
Author
|
After checking the implementation (i.e. the test zone files and CoreDNS configuration), I’ve brought the spec in line with how it was actually implemented. It doesn’t change the core idea of the scenario, however. |
marc-vanderwal
added a commit
to marc-vanderwal/zonemaster-engine
that referenced
this pull request
Jul 29, 2025
Another rewrite of a unit test that led me to uncover a typo in the original unit test. Actually, this time, the specification was incorrect (see zonemaster/zonemaster#1414).
marc-vanderwal
added a commit
to marc-vanderwal/zonemaster-engine
that referenced
this pull request
Jul 30, 2025
Another rewrite of a unit test that led me to uncover a typo in the original unit test. Actually, this time, the specification was incorrect (see zonemaster/zonemaster#1414).
marc-vanderwal
added a commit
to marc-vanderwal/zonemaster-engine
that referenced
this pull request
Jul 30, 2025
Another rewrite of a unit test that led me to uncover a typo in the original unit test. Actually, this time, the specification was incorrect (see zonemaster/zonemaster#1414).
matsduf
reviewed
Jul 30, 2025
matsduf
previously approved these changes
Jul 30, 2025
Contributor
Author
|
I noticed another oddity: in the CoreDNS configuration for this scenario, there are Shouldn’t these I’m not sure if this is an error, because making the changes on my local setup does not change the messages that my runs of |
Contributor
Author
|
All is well. It turns out I needed to hunt down typos in more places that I thought. Now the scenario works again. I’ll make a PR that rerecords |
marc-vanderwal
added a commit
to marc-vanderwal/zonemaster-engine
that referenced
this pull request
Jul 31, 2025
Scenario MULT-SOA-MNAMES-NO-DEL-UNDEL-2 was marked as not testable, but it turned out that it was due to errors in the test scenario implementation (i.e. the zone files and CoreDNS configuration). After fixing the problem (see zonemaster/zonemaster#1414), we can enable the scenario in Zonemaster-Engine’s test suite. The test data is also rerecorded.
Contributor
No. "ns1" and "ns2" are IB names in the thought delegation not present in the undelegated zone. Note NXDOMAIN. "ns3" and "ns4" are OOB and cannot belong to the zone. |
matsduf
requested changes
Aug 1, 2025
Looking carefully at the logs generated by CoreDNS shows that there were some issues with the zone files and the configuration that prevented that scenario from actually working correctly. Fortunately, fixing these typos now fixes the zonemaster-cli run.
Caught a few more typos in Consistency06 test scenario specifications.
b3b7dbf to
f383f69
Compare
marc-vanderwal
added a commit
to marc-vanderwal/zonemaster-engine
that referenced
this pull request
Aug 4, 2025
Scenario MULT-SOA-MNAMES-NO-DEL-UNDEL-2 was marked as not testable, but it turned out that it was due to errors both in the test scenario implementation (i.e. the zone files and CoreDNS configuration: see zonemaster/zonemaster#1414) and a small typo in the .t file. In the .t file, scenario MULT-SOA-MNAMES-NO-DEL-UNDEL-2 erroneously listed ns3.mult-soa-mnames-no-del-undel-2.consistency06.xb twice in its undelegated data. The second instance should be ns4, not ns3. After fixing these problems, we can enable the scenario in Zonemaster-Engine’s test suite and rerecord the test data.
matsduf
approved these changes
Aug 5, 2025
marc-vanderwal
added a commit
to marc-vanderwal/zonemaster-engine
that referenced
this pull request
Aug 7, 2025
Another rewrite of a unit test that led me to uncover a typo in the original unit test. Actually, this time, the specification was incorrect (see zonemaster/zonemaster#1414).
marc-vanderwal
added a commit
to marc-vanderwal/zonemaster-engine
that referenced
this pull request
Aug 7, 2025
Another rewrite of a unit test that led me to uncover a typo in the original unit test. Actually, this time, the specification was incorrect (see zonemaster/zonemaster#1414).
tgreenx
approved these changes
Oct 7, 2025
marc-vanderwal
added a commit
to marc-vanderwal/zonemaster-engine
that referenced
this pull request
Oct 15, 2025
Another rewrite of a unit test that led me to uncover a typo in the original unit test. Actually, this time, the specification was incorrect (see zonemaster/zonemaster#1414).
marc-vanderwal
added a commit
to marc-vanderwal/zonemaster-engine
that referenced
this pull request
Oct 15, 2025
Another rewrite of a unit test that led me to uncover a typo in the original unit test. Actually, this time, the specification was incorrect (see zonemaster/zonemaster#1414).
marc-vanderwal
added a commit
to marc-vanderwal/zonemaster-engine
that referenced
this pull request
Oct 16, 2025
Another rewrite of a unit test that led me to uncover a typo in the original unit test. Actually, this time, the specification was incorrect (see zonemaster/zonemaster#1414).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
This PR fixes a handful of minor errors in a test scenario for Consistency06, in the specification and the configuration files that implement it. Together, they prevented the correct operation of scenario MULT-SOA-MNAMES-NO-DEL-UNDEL-2.
Context
Refactoring of Zonemaster-Engine unit tests.
Changes
zonemaster-clioutput.How to test this PR
N/A.