Adds new unit tests based on scenarios for Basic02#1447
Conversation
Co-authored-by: Marc van der Wal <103426270+marc-vanderwal@users.noreply.github.com>
| my $zone; | ||
|
|
||
| %res = map { $_->tag => 1 } Zonemaster::Engine->test_module( q{basic}, q{nic.tf} ); | ||
| ok( $res{B02_AUTH_RESPONSE_SOA}, q{B02_AUTH_RESPONSE_SOA} ); |
There was a problem hiding this comment.
This could have been kept, no? Since it tests the entire Basic module.
There was a problem hiding this comment.
It could, but the new unit tests does much more, and I think we should clean out old unit tests when we add new ones based on scenarios.
There was a problem hiding this comment.
Testing a test case and a test module are two different things. In particular, calling Zonemaster::Engine->test_module() (as opposed to Zonemaster::Engine->test_method() for a single test case which is what TestUtil->perform_testcase_testing() does) will run the all() method in that module (which takes care of any test case dependencies). So it remains important to have those unit tests, we should keep them.
There was a problem hiding this comment.
We can keep it. It will do no harm until we need to re-record and it does not work. A better approach wold be to design tests unsing Zonemaster::Engine->test_module() in the test framework, if that really adds something.
Please note that the tests on Basic01 have already been removed when new Basic01 tests were created.
Co-authored-by: tgreenx <96772376+tgreenx@users.noreply.github.com>
Co-authored-by: tgreenx <96772376+tgreenx@users.noreply.github.com>
Updates from "single_scenario" to "selected_scenario" and from "disable_scenario" to "disabled_scenarios" to better match the function. No change of logic.
|
@tgreenx and @marc-vanderwal, please review. |
| my $zone; | ||
|
|
||
| %res = map { $_->tag => 1 } Zonemaster::Engine->test_module( q{basic}, q{nic.tf} ); | ||
| ok( $res{B02_AUTH_RESPONSE_SOA}, q{B02_AUTH_RESPONSE_SOA} ); |
There was a problem hiding this comment.
Testing a test case and a test module are two different things. In particular, calling Zonemaster::Engine->test_module() (as opposed to Zonemaster::Engine->test_method() for a single test case which is what TestUtil->perform_testcase_testing() does) will run the all() method in that module (which takes care of any test case dependencies). So it remains important to have those unit tests, we should keep them.
Co-authored-by: tgreenx <96772376+tgreenx@users.noreply.github.com>
| # - One of MANDATORY_MESSAGE_TAGS and FORBIDDEN_MESSAGE_TAGS may be undefined. | ||
| # See documentation for the meaning of that. | ||
|
|
||
| my %subtests = ( |
There was a problem hiding this comment.
Indentation of this block is still looking weird imho (compare with e.g. Test-basic01.t), but I could live with it.
The pull request zonemaster#1447 renamed ZONEMASTER_SINGLE_SCENARIO to ZONEMASTER_SELECTED_SCENARIOS and ZONEMASTER_DISABLE_SCENARIO to ZONEMASTER_DISABLED_SCENARIOS. This PR has already been merged, so this instance needs to be updated as well.
Purpose
This PR adds new unit tests for Basic02. They are based on the defined scenarios in zonemaster/zonemaster#1380.
Old unit tests are removed in this PR, and data file rerecorded.
The
TestUtil.pmPerl module for unit tests is also updated. The update is backward compatible. The update adds flexibility to make it possible to test multiple, selected scenarios during development.To resolve the issue in #1448, TestUtil.pm is updated. The "fill_in_empty_oob_glue"
flag to add_fake_delegation()is changed fromfalsetotrue.All data files that belong to unit tests that use TestUtil.pm have been updated after the change, except for the data file for
t/Test-zone11.t. The source data for those unit tests are on a private network not available for the creator of this PR.Context
zonemaster/zonemaster#1380
How to test this PR
The unit tests must pass.