Skip to content

Updates methodsv2 unit tests#1351

Merged
matsduf merged 12 commits intozonemaster:developfrom
matsduf:updates-methodsv2-unit-tests
Jul 23, 2024
Merged

Updates methodsv2 unit tests#1351
matsduf merged 12 commits intozonemaster:developfrom
matsduf:updates-methodsv2-unit-tests

Conversation

@matsduf
Copy link
Contributor

@matsduf matsduf commented May 27, 2024

Purpose

This PR provides unit tests based on test scenarios for MethodsV2, as defined in zonemaster/zonemaster#1254. It also updates t/TestUtil.pm.

  • Updates of t/TestUtil.pm:

    • Explicit error messages expected list does not match extracted by code
    • Support for distrinktion between empty list and undefined list
    • Added feature "single scenario"
    • Added feature "disable scenarios" for one run
    • Corrected bug that created undefined IP address when IP addresses were extracted from name/IP expressions an expression was name only
    • Replaced built in IP address verification against from Zonemaster library
  • Updates of t/methodsv2.t:

    • Adds more scenarios/unit tests
    • Updates unit tests
    • Adds support for running a single scenario or temporarily disable a scenario or list of scenarios
  • Updated t/methodsv2.data

How to test

All unit tests should be enabled and all should pass. For that the result of #1373 must be installed.

@matsduf matsduf marked this pull request as draft May 27, 2024 21:55
@tgreenx tgreenx added this to the v2024.2 milestone Jun 11, 2024
@matsduf matsduf force-pushed the updates-methodsv2-unit-tests branch 2 times, most recently from 58a034e to 7b12a5c Compare July 9, 2024 21:37
@matsduf matsduf requested a review from tgreenx July 9, 2024 21:39
@matsduf matsduf force-pushed the updates-methodsv2-unit-tests branch 2 times, most recently from 1747b77 to fb3ca78 Compare July 9, 2024 22:22
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 so far

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.

I suggest the following fix to handle undefined values in the input data hash :

diff --git a/t/TestUtil.pm b/t/TestUtil.pm
index 2f627823..28941126 100644
--- a/t/TestUtil.pm
+++ b/t/TestUtil.pm
@@ -334,9 +334,9 @@ sub perform_methodsv2_testing {

             # Methods: get_del_ns_names() and get_zone_ns_names()
             @method_names = qw( get_del_ns_names get_zone_ns_names );
-            my @expected_del_ns_names = uniq map { (split( m(/), $_ ))[0] } @{ $expected_del_ns };
-            my @expected_zone_ns_names = uniq map { (split( m(/), $_ ))[0] } @{ $expected_zone_ns };
-            my @expected_ns_names = ( \@expected_del_ns_names, \@expected_zone_ns_names );
+            my $expected_del_ns_names = defined $expected_del_ns ? [ uniq map { (split( m(/), $_ ))[0] } @{ $expected_del_ns } ] : undef;
+            my $expected_zone_ns_names = defined $expected_zone_ns ? [ uniq map { (split( m(/), $_ ))[0] } @{ $expected_zone_ns } ] : undef;
+            my @expected_ns_names = ( $expected_del_ns_names, $expected_zone_ns_names );
             foreach my $i ( 0..$#method_names ) {
                 my $method = $method_names[$i];
                 subtest $method => sub {
@@ -362,9 +362,9 @@ sub perform_methodsv2_testing {

             # Methods: get_del_ns_ips() and get_zone_ns_ips()
             @method_names = qw( get_del_ns_ips get_zone_ns_ips );
-            my @expected_del_ns_ips = grep defined, uniq map { (split( m(/), $_ ))[1] } @{ $expected_del_ns };
-            my @expected_zone_ns_ips = grep defined, uniq map { (split( m(/), $_ ))[1] } @{ $expected_zone_ns };
-            my @expected_ns_ips = ( \@expected_del_ns_ips, \@expected_zone_ns_ips );
+            my $expected_del_ns_ips = defined $expected_del_ns ? [ uniq map { (split( m(/), $_ ))[1] } @{ $expected_del_ns } ] : undef;
+            my $expected_zone_ns_ips = defined $expected_zone_ns ? [ uniq map { (split( m(/), $_ ))[1] } @{ $expected_zone_ns } ] : undef;
+            my @expected_ns_ips = ( $expected_del_ns_ips, $expected_zone_ns_ips ); 

Moreover I suggest that some new checks be added around line 269 to forbid the presence of any undefined value in the arrays of the input data hash.

@matsduf
Copy link
Contributor Author

matsduf commented Jul 11, 2024

I suggest the following fix to handle undefined values in the input data hash :

I implemented the proposal and ran one unit test under the latest code of #1373 and get the following output (still with defined array in methods.t):

        #   Failed test 'Result is defined'
        #   at /home/ubuntu/git/zonemaster-engine/t/TestUtil.pm line 297.
        # Unexpected undefined result
        # Looks like you failed 1 test of 1.

    #   Failed test 'get_parent_ns_ips'
    #   at /home/ubuntu/git/zonemaster-engine/t/TestUtil.pm line 307.
    # Looks like you failed 1 test of 1.
t/methodsv2.t .. 4/? 
#   Failed test 'NO-CHILD-2'
#   at /home/ubuntu/git/zonemaster-engine/t/TestUtil.pm line 395.
Can't use an undefined value as an ARRAY reference at /home/ubuntu/git/zonemaster-engine/t/TestUtil.pm line 302.

It is hard to understand such a message. It should be clearer. It is not obvious what it means.

If I add undef in methods.t there is no error.

All tests successful.

@matsduf
Copy link
Contributor Author

matsduf commented Jul 11, 2024

Moreover I suggest that some new checks be added around line 269 to forbid the presence of any undefined value in the arrays of the input data hash.

I do not know what you suggest.

@matsduf matsduf force-pushed the updates-methodsv2-unit-tests branch from 83211a7 to 3ba6d01 Compare July 12, 2024 19:57
@matsduf matsduf marked this pull request as ready for review July 12, 2024 20:08
Copy link
Contributor

@marc-vanderwal marc-vanderwal 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 to me, save for a few typos.

matsduf and others added 5 commits July 16, 2024 18:50
* Explicit error messages expected list does not match extracted by code
* Support for distrinktion between empty list and undefined list
* Added feature "single scenario"
* Added feature "disable scenarios" for one run
* Corrected bug that created undefined IP address when IP addresses were
  extracted from name/IP expressions an expression was name only
* Replaced built in IP address verification against from Zonemaster library
* Adds more scenarios/unit tests
* Updates unit tests
* Adds support for running a single scenario or
  temporarily disable a scenario or list of scenarios
Co-authored-by: Marc van der Wal <103426270+marc-vanderwal@users.noreply.github.com>
- Fix support of name servers without IP addresses in input data hash
- Update unit tests
- Update unit test data
@matsduf matsduf requested a review from marc-vanderwal July 16, 2024 19:39
tgreenx added 2 commits July 17, 2024 13:33
- Fix support of name servers without IP address
- Add check for existence of single scenario
- Update unit test and unit test data
tgreenx
tgreenx previously approved these changes Jul 17, 2024
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.

@matsduf I took the liberty of adding a few commits in this PR. Now, together with the latest changes in #1373, all unit tests pass.

Co-authored-by: Marc van der Wal <103426270+marc-vanderwal@users.noreply.github.com>
marc-vanderwal
marc-vanderwal previously approved these changes Jul 18, 2024
@matsduf matsduf added the V-Patch Versioning: The change gives an update of patch in version. label Jul 18, 2024
tgreenx
tgreenx previously approved these changes Jul 23, 2024
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.

@matsduf I just merged #1373 so you can rebase this one on latest develop.

@matsduf matsduf dismissed stale reviews from tgreenx and marc-vanderwal via f5cd61c July 23, 2024 13:39
@matsduf
Copy link
Contributor Author

matsduf commented Jul 23, 2024

@matsduf I just merged #1373 so you can rebase this one on latest develop.

@tgreenx, I have merged develop on this PR.

@matsduf
Copy link
Contributor Author

matsduf commented Jul 23, 2024

All tests pass. All comments handled. Has previously been approved by @marc-vanderwal

@matsduf matsduf merged commit b814356 into zonemaster:develop Jul 23, 2024
@matsduf matsduf deleted the updates-methodsv2-unit-tests branch July 23, 2024 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

V-Patch Versioning: The change gives an update of patch in version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants