Updates unit tests for ZONE09

[matsduf]

Purpose

This PR replaces the unit tests for ZONE09 with unit tests and data based on the Test Zone framework proposed in
zonemaster/zonemaster#1151. All tests are described in that PR.

How to test this PR

The unit tests must pass the CI checks.

Review the set-up of the code.

[marc-vanderwal]

Looks good to me.

I like the idea of the test being described by data following the __DATA__ keyword. Though maybe this structured data could be expanded to become some kind of domain-specific language, and become a bit more self-documenting in the process.

I was thinking about a syntax similar to the following:

zone no-response-mx-query.zone09.xa
should give Z09_NO_RESPONSE_MX_QUERY
# and listing message tags that should not appear becomes optional

zone inconsistent-mx.zone09.xa
should give Z09_INCONSISTENT_MX, Z09_MX_FOUND, Z09_NO_MX_FOUND, Z09_MX_DATA
should not give Z09_MISSING_MAIL_TARGET

We might also want to give ourselves a way to continue a list of message tags on multiple lines, in order to avoid excessively long lines.

But my suggestion is better addressed in another PR.

[matsduf]

I like the idea of the test being described by data following the __DATA__ keyword. Though maybe this structured data could be expanded to become some kind of domain-specific language, and become a bit more self-documenting in the process.

I see pros of your suggestions, but then I think should be keys that stick out more, and could even be abbreviations, and that could permit multiple physical lines for one logical. E.g.

#ZN: inconsistent-mx.zone09.xa
#GS: Z09_INCONSISTENT_MX, Z09_MX_FOUND, Z09_NO_MX_FOUND, Z09_MX_DATA
#GN: Z09_MISSING_MAIL_TARGET

But my suggestion is better addressed in another PR.

Yes, I think so. This file could be updated when we have found the format.

[marc-vanderwal]

I’ve spent an afternoon tinkering with a parser for a small language that accepts a syntax like this:

zone inconsistent-mx.zone09.xa
    should give Z09_INCONSISTENT_MX, Z09_MX_FOUND, Z09_NO_MX_FOUND and Z09_MX_DATA
    should not give Z09_MISSING_MAIL_TARGET

Whitespace is optional everywhere and has no syntactic value; in other words, indentation is entirely optional and the only way to terminate a zone declaration is to start another one or to reach the end of the file. An and is equivalent to a comma.

I drew inspiration from the syntax of a .fetchmailrc file. It can be parsed with an LALR grammar such as those used in lex/yacc or `Parse::RecDescent. The language has room for extension, for example if we want to test not only the presence of a given message, but the value of one or more of its arguments.

[matsduf]

I’ve spent an afternoon tinkering with a parser for a small language that accepts a syntax like this:

zone inconsistent-mx.zone09.xa
    should give Z09_INCONSISTENT_MX, Z09_MX_FOUND, Z09_NO_MX_FOUND and Z09_MX_DATA
    should not give Z09_MISSING_MAIL_TARGET

You do not like my suggestion of e.g. #ZN:, #GS: and #GN: as the keys? I think those would be easier to identify, not for the code but for the human.

Whitespace is optional everywhere and has no syntactic value; in other words, indentation is entirely optional and the only way to terminate a zone declaration is to start another one or to reach the end of the file.

That would be fine.

An and is equivalent to a comma.

I am not sure that is a good idea. Then should not give Z09_MISSING_MAIL_TARGET and Z09_NULL_MX_NON_ZERO_PREF could be misleading since it should rather be or in normal language. There is no reason to have extra and-suger.

I drew inspiration from the syntax of a .fetchmailrc file. It can be parsed with an LALR grammar such as those used in lex/yacc or `Parse::RecDescent. The language has room for extension, for example if we want to test not only the presence of a given message, but the value of one or more of its arguments.

If we agree on the syntax, could you implement that in a utility pm file? But that would be for the v2023.2 release, wouldn't it?

[marc-vanderwal]

Your idea works, and makes for a much simpler parser. However, abbreviations such as ZN, GS and GN can seem a bit cryptic, especially for those who are unfamiliar with the codebase. Would a DSL with a full-blown LALR grammar be overkill? I don’t think so: because we are going to use it extensively in our test suite, I believe that it’s worth spending time to play around with Perl’s equivalent of lex/yacc (although it seems to be the Brussels’ sprouts of computing).

I understand your concern with having and be a synonym for ,, as it can indeed be misleading in a should not give clause. For the time being, and can be left out from the grammar.

Anyhow, no attempt at a nicer language for describing these classes of tests will make it for the 2023.1 release. Unless anyone else wishes to look at this PR, I suggest this one be merged as it is. 🙂

[tgreenx]

LGTM. I just have one concern when a Test Case is not tested by the whole module, see #1216 (comment).

[matsduf]

LGTM. I just have one concern when a Test Case is not tested by the whole module, see #1216 (comment).

This PR does not affect any component based t file. Could you elaborate?

[tgreenx]

LGTM. I just have one concern when a Test Case is not tested by the whole module, see #1216 (comment).

This PR does not affect any component based t file. Could you elaborate?

See #1216 (comment)