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

Update unit tests for Consistency05 and Consistency06 #1303

Merged

Conversation

matsduf
Copy link
Contributor

@matsduf matsduf commented Nov 13, 2023

Purpose

The PR creates unit tests based on the new test zone framework. The unit tests in this PR is based on the data in zonemaster/zonemaster#1213. It also removes old unit tests.

Due to a bug in the implementation of Constistency05 and Consistency06, respectively, some scenarios cannot presently be used. The following scenarios have not been included as unit tests due to the bug:

  • CHILD-ZONE-LAME-1 (Consistency05)
  • IB-ADDR-MISMATCH-3 (Consistency05)
  • ONE-SOA-MNAME-4 (Consistency06)
  • NO-RESPONSE (Consistency06)

The bug is described in #1300 and #1301.

Changes

New unit tests are added in t/Test-consistency05.t and t/Test-consistency06.t (plus data files).

Old unit tests for Consistency05 and Consistency06 are removed, except for t/Test-consistency05-F.t and t/Test-consistency06-B.t that have been kept since they test for the presence of NO_RESPONSE (with the help of "fake delegation").

The MANIFEST file has also been updated.

How to test this PR

Review zonemaster/zonemaster#1213 and verify that the CI passes.

@matsduf matsduf added the A-TestCase Area: Test case specification or implementation of test case label Nov 13, 2023
@matsduf matsduf added this to the v2023.2 milestone Nov 13, 2023
@matsduf matsduf force-pushed the update-unit-tests-consistency05-06 branch from ebbab4f to 6bf1e01 Compare November 19, 2023 14:03
@matsduf matsduf force-pushed the update-unit-tests-consistency05-06 branch from 6bf1e01 to 158202c Compare November 19, 2023 14:53
@matsduf
Copy link
Contributor Author

matsduf commented Nov 19, 2023

@tgreenx, can you review this and also the underlying zonemaster/zonemaster#1213?

@matsduf
Copy link
Contributor Author

matsduf commented Nov 27, 2023

@marc-vanderwal and @tgreenx, can you review this?

Copy link
Contributor

@tgreenx tgreenx left a comment

Choose a reason for hiding this comment

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

LGTM. I just have one, non-blocking comment.

# Scenarios CHILD-ZONE-LAME-1 and IB-ADDR-MISMATCH-3 cannot be tested due to a bug in the implementation. See
# https://github.com/zonemaster/zonemaster-engine/issues/1301
#
my %subtests = (
Copy link
Contributor

@tgreenx tgreenx Nov 29, 2023

Choose a reason for hiding this comment

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

For easier reviewing it would be best if the scenario keys in this hash were ordered in the same way as in the specification.

The same applies for file t/Test-consistency06.t below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I really agree. This an artefact of my script that converts a specification to a t file. I just created a Perl hash and took the order keys gave. I will update the script to get the same order as the specification.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will fix that afterwards.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will fix that in a new PR.

@matsduf matsduf merged commit fbd5fd7 into zonemaster:develop Dec 4, 2023
3 checks passed
@matsduf matsduf deleted the update-unit-tests-consistency05-06 branch December 4, 2023 09:02
@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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants