Skip to content

Update unit tests for Consistency05 and Consistency06#1303

Merged
matsduf merged 3 commits into
zonemaster:developfrom
matsduf:update-unit-tests-consistency05-06
Dec 4, 2023
Merged

Update unit tests for Consistency05 and Consistency06#1303
matsduf merged 3 commits into
zonemaster:developfrom
matsduf:update-unit-tests-consistency05-06

Conversation

@matsduf

@matsduf matsduf commented Nov 13, 2023

Copy link
Copy Markdown
Contributor

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

matsduf commented Nov 19, 2023

Copy link
Copy Markdown
Contributor Author

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

@matsduf

matsduf commented Nov 27, 2023

Copy link
Copy Markdown
Contributor Author

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

@tgreenx tgreenx left a comment

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.

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

Comment thread t/Test-consistency05.t
# 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 = (

@tgreenx tgreenx Nov 29, 2023

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.

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
@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