Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactoring of utility function in Zonemaster::Engine::Test::Zone #1296

Merged
merged 1 commit into from
Oct 10, 2023

Conversation

tgreenx
Copy link
Contributor

@tgreenx tgreenx commented Sep 20, 2023

Purpose

This PR updates the aforementioned module (Zonemaster::Engine::Test::Zone) with respect to other Test modules, for consistency. See the Changes section below.

Context

Fixes #1276

Changes

  • Rename _is_ip_version_disabled() to _ip_disabled_message()
  • Replace message tags SKIP_IPV{4/6}_DISABLED with IPV{4/6}_DISABLED

How to test this PR

Tests should pass.

Also, IPV{4/6}_DISABLED message tags are now shown:
(Note that the remaining SKIP_IPV6_DISABLED messages are from direct calls to the Zonemaster::Engine::Zone module, and are now properly distinguishable in this Test module)

$ zonemaster-cli --show-testcase --test=zone --no-ipv6 --raw --level=debug 'zonemaster.net' | grep -E 'TEST_CASE_START|IPV6_DISABLED'
   0.00 DEBUG    ZONE01         TEST_CASE_START   testcase=zone01
   1.19 DEBUG    ZONE01         IPV6_DISABLED   ns=ns2.nic.fr/2001:660:3005:1::1:2; rrtype=SOA
   1.21 DEBUG    ZONE01         IPV6_DISABLED   ns=nsa.dnsnode.net/2a01:3f1:46::53; rrtype=SOA
   1.22 DEBUG    ZONE01         IPV6_DISABLED   ns=nsp.dnsnode.net/2a01:3f1:3032::53; rrtype=SOA
   1.25 DEBUG    ZONE01         IPV6_DISABLED   ns=nsu.dnsnode.net/2a01:3f0:400::32; rrtype=SOA
   1.26 DEBUG    ZONE01         SKIP_IPV6_DISABLED   ns=ns2.nic.fr/2001:660:3005:1::1:2; rrtype=NS
   1.26 DEBUG    ZONE01         SKIP_IPV6_DISABLED   ns=nsa.dnsnode.net/2a01:3f1:46::53; rrtype=NS
   1.26 DEBUG    ZONE01         SKIP_IPV6_DISABLED   ns=nsp.dnsnode.net/2a01:3f1:3032::53; rrtype=NS
   1.26 DEBUG    ZONE01         SKIP_IPV6_DISABLED   ns=nsu.dnsnode.net/2a01:3f0:400::32; rrtype=NS
   1.33 DEBUG    ZONE01         IPV6_DISABLED   ns=nsa.dnsnode.net/2a01:3f1:46::53; rrtype=SOA
   1.33 DEBUG    ZONE02         TEST_CASE_START   testcase=zone02
   1.33 DEBUG    ZONE03         TEST_CASE_START   testcase=zone03
   1.33 DEBUG    ZONE04         TEST_CASE_START   testcase=zone04
   1.33 DEBUG    ZONE05         TEST_CASE_START   testcase=zone05
   1.33 DEBUG    ZONE06         TEST_CASE_START   testcase=zone06
   1.33 DEBUG    ZONE07         TEST_CASE_START   testcase=zone07
   1.34 DEBUG    ZONE08         TEST_CASE_START   testcase=zone08
   1.36 DEBUG    ZONE08         SKIP_IPV6_DISABLED   ns=ns2.nic.fr/2001:660:3005:1::1:2; rrtype=CNAME
   1.38 DEBUG    ZONE08         SKIP_IPV6_DISABLED   ns=ns2.nic.fr/2001:660:3005:1::1:2; rrtype=CNAME
   1.39 DEBUG    ZONE09         TEST_CASE_START   testcase=zone09
   1.39 DEBUG    ZONE09         IPV6_DISABLED   ns=ns2.nic.fr/2001:660:3005:1::1:2; rrtype=SOA
   1.39 DEBUG    ZONE09         IPV6_DISABLED   ns=ns2.nic.fr/2001:660:3005:1::1:2; rrtype=MX
   1.40 DEBUG    ZONE09         IPV6_DISABLED   ns=nsa.dnsnode.net/2a01:3f1:46::53; rrtype=SOA
   1.40 DEBUG    ZONE09         IPV6_DISABLED   ns=nsa.dnsnode.net/2a01:3f1:46::53; rrtype=MX
   1.42 DEBUG    ZONE09         IPV6_DISABLED   ns=nsp.dnsnode.net/2a01:3f1:3032::53; rrtype=SOA
   1.42 DEBUG    ZONE09         IPV6_DISABLED   ns=nsp.dnsnode.net/2a01:3f1:3032::53; rrtype=MX
   1.46 DEBUG    ZONE09         IPV6_DISABLED   ns=nsu.dnsnode.net/2a01:3f0:400::32; rrtype=SOA
   1.46 DEBUG    ZONE09         IPV6_DISABLED   ns=nsu.dnsnode.net/2a01:3f0:400::32; rrtype=MX
   1.46 DEBUG    ZONE10         TEST_CASE_START   testcase=zone10
   1.46 DEBUG    ZONE10         IPV6_DISABLED   ns=ns2.nic.fr/2001:660:3005:1::1:2; rrtype=SOA
   1.46 DEBUG    ZONE10         IPV6_DISABLED   ns=nsa.dnsnode.net/2a01:3f1:46::53; rrtype=SOA
   1.46 DEBUG    ZONE10         IPV6_DISABLED   ns=nsp.dnsnode.net/2a01:3f1:3032::53; rrtype=SOA
   1.46 DEBUG    ZONE10         IPV6_DISABLED   ns=nsu.dnsnode.net/2a01:3f0:400::32; rrtype=SOA

This commit aligns the aforementioned module with other Test modules for consistency.

Rename '_is_ip_version_disabled()' to '_ip_disabled_message()'
Replace message tags 'SKIP_IPV{4/6}_DISABLED' with 'IPV{4/6}_DISABLED'
@tgreenx tgreenx added A-TestCase Area: Test case specification or implementation of test case V-Minor Versioning: The change gives an update of minor in version. labels Sep 20, 2023
@tgreenx tgreenx added this to the v2023.2 milestone Sep 20, 2023
@tgreenx tgreenx linked an issue Sep 20, 2023 that may be closed by this pull request
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Looks good. Just a remark that might be outside of scope of this PR: the message tags SKIP_IPVx_DISABLED are linked to the testcase (here ZONE0x in your example). Can we consider moving them to SYSTEM or something else since they are not directly linked to the testcase (not defined in the specification)?
(Maybe I should open an issue for this.)

@matsduf
Copy link
Contributor

matsduf commented Sep 28, 2023

Looks good. Just a remark that might be outside of scope of this PR: the message tags SKIP_IPVx_DISABLED are linked to the testcase (here ZONE0x in your example). Can we consider moving them to SYSTEM or something else since they are not directly linked to the testcase (not defined in the specification)? (Maybe I should open an issue for this.)

They are still related to the execution of the test case. I think it is relevant to bind all messages that are outputted within the execution of the test case to the test case. If a message will not be outputted if the test case is removed from the profile should probably be tagged for that test case.

Note that also TEST_CASE_START and TEST_CASE_END are not defined in the specification.

@matsduf
Copy link
Contributor

matsduf commented Sep 28, 2023

It is also noticeable that for several test cases no IPV6_DISABLED is outputted.

@tgreenx
Copy link
Contributor Author

tgreenx commented Sep 28, 2023

It is also noticeable that for several test cases no IPV6_DISABLED is outputted.

Yes that is normal from the current implementation. Test Cases Zone02 to Zone07 call a special helper function _retrieve_record_from_zone(), which, as stated in the documentation from the current develop branch, reads:

Retrieves resource records of given type for the given name from the response of the first authoritative server of the given zone that has at least one.

Considering that the first queried name server is IPv4 and gives the desired resource record(s), no other query is sent (meaning that no IPv6 name server is skipped thus leading to no IPV6_DISABLED messages for those Test Cases)

@tgreenx tgreenx merged commit 681c717 into zonemaster:develop Oct 10, 2023
3 checks passed
@tgreenx tgreenx deleted the update-zone branch October 10, 2023 13:50
@hannaeko hannaeko self-assigned this Jan 10, 2024
@hannaeko hannaeko added the S-ReleaseTested Status: The PR has been successfully tested in release testing label Jan 10, 2024
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 S-ReleaseTested Status: The PR has been successfully tested in release testing V-Minor Versioning: The change gives an update of minor in version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incoherent helper function name and message tags in Zone test module
3 participants