Skip to content

Conversation

@marc-vanderwal
Copy link
Contributor

@marc-vanderwal marc-vanderwal commented Jan 8, 2026

Purpose

This PR removes t/old-bugs.t and its associated data file from the test suite.

These tests are very obsolete and depend on live data returned by name servers on the Internet. Because of that, much of the data cannot be rerecorded anymore. The tests in question are also largely superseded by the newer scenario-based tests.

Deleting these files does not reduce the coverage of the test suite in general, except for two very specific places in the entire codebase of Zonemaster-Engine that probably need to be overhauled anyway.

After careful analysis, it turns out that the loss of coverage is the consequence of unclear or incomplete specifications and it would make more sense to rework the specifications and their implementations instead of adding band-aids to keep the coverage identical. The following issues were created in that context:

Context

While developing some of the features and fixes for the latest release (v2025.2), we’ve had problems due to some .t files that were more or less difficult to fix because rerecording the corresponding data file was not always doable. Sometimes, I had to resort to extreme kludges to .data files to get tests passing again. One example is commit e24a21e, part of #1475.

And t/old-bugs.data is a prime example of a data file containing things that cannot ever be rerecorded, again, because the file is old and the Internet has moved on in the meantime.

Changes

  • Remove t/old-bugs.t and t/old-bugs.data.

How to test this PR

Unit tests should still pass and coverage should stay unchanged, save for the two aforementioned places.

With Devel::Cover installed, generate a coverage report on the codebase while t/old-bugs.t and .data are still there:

$ cover -test cover_db.before

Then check out this branch and generate another report:

$ cover -test cover_db.after

Open the cover_db.before/coverage.html and cover_db.after/coverage.html files side by side. The only difference you should see is before:
image
and after (differences circled in red):
image

Open cover_db.{before,after}/blib-lib-Zonemaster-Engine-Test-Delegation-pm--branch.html side by side. The only difference should be at line 502:
image
(The circled area is green in the “before” report and red in the “after” report).

Open http://localhost:8080/cover_db.after/blib-lib-Zonemaster-Engine-Test-Nameserver-pm--condition.html side by side. The only difference should be at line 700:
image
(The area circled in red is green in the “before” report and red in the “after” report).

These tests are very obsolete and depend on live data returned by name
servers on the Internet. Because of that, they cannot be rerecorded
anymore. The tests in question are also largely superseded by the newer
scenario-based tests.

Deleting these files does not affect the coverage of the test suite in
general, except for two very specific places in the entire codebase of
Zonemaster-Engine.

After careful analysis, it turns out that restoring that coverage by
means of scenarios isn’t trivial. The affected code implements the
Delegation03 and Nameserver01 test cases. The associated specifications
will need some rework and it is likely that the code will be entirely
overhauled in the future.
@marc-vanderwal marc-vanderwal added this to the v2026.1 milestone Jan 8, 2026
@marc-vanderwal marc-vanderwal added the RC-None Release category: Not to be included in Changes file. label Jan 8, 2026
Copy link
Contributor

@matsduf matsduf left a comment

Choose a reason for hiding this comment

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

Even if we loose some coverage in the short run, I think it is much better to focus on the new scenario model and remove legacy tests that we cannot control

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

RC-None Release category: Not to be included in Changes file.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants