Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose Zonemaster-LDNS version #1050

Merged
1 commit merged into from Sep 5, 2022
Merged

Expose Zonemaster-LDNS version #1050

1 commit merged into from Sep 5, 2022

Conversation

ghost
Copy link

@ghost ghost commented Aug 25, 2022

Purpose

This exposes the Zonemaster-LDNS version via the API call version_info.

Context

Addresses #790 (comment)

Changes

Update version_info to expose Zonemaster-LDNS version.
Update API.md document.
Update t/test01.t to check that version_info returns a value for Zonemaster-LDNS.

How to test this PR

Make a call to version_info and see that the Zonemaster-LDNS version is exposed.

zmb version_info | jq

@ghost ghost added this to the v2022.2 milestone Aug 25, 2022
@ghost ghost requested review from mattias-p, matsduf and hannaeko August 25, 2022 09:47
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.

It looks fine, but travis fails.

@ghost
Copy link
Author

ghost commented Aug 25, 2022

This comes from the PR zonemaster/zonemaster-engine#1117 that removes some messages from the INFO level. But the test relies on such message to assert if IPv6 is disabled:

subtest 'second test has IPv6 disabled' => sub {
my $res = $rpcapi->get_test_params( { test_id => $hash_id } );
is( $res->{ipv4}, $params->{ipv4}, 'Retrieve the correct "ipv4" value' );
is( $res->{ipv6}, $params->{ipv6}, 'Retrieve the correct "ipv6" value' );
$res = $rpcapi->get_test_results( { id => $hash_id, language => 'en_US' } );
my @msg_basic = map { $_->{message} if $_->{module} eq 'BASIC' } @{ $res->{results} };
ok( grep( /IPv6 is disabled/, @msg_basic ), 'Results contain an "IPv6 is disabled" message' );
};

@matsduf
Copy link
Contributor

matsduf commented Aug 25, 2022

This comes from the PR zonemaster/zonemaster-engine#1117 that removes some messages from the INFO level. But the test relies on such message to assert if IPv6 is disabled:

When zonemaster/zonemaster#1090 is approved, merged and implemented there is a new message to look for. This PR could maybe wait for that to be done?

@ghost
Copy link
Author

ghost commented Aug 29, 2022

The issue is not related to this PR but from how the code evolved in Zonemaster-Engine. All new PR will fail in Backend. To solve this we could either:

  1. postpone all PR in Backend until Update basic04 zonemaster#1090 is approved, merged and implemented
  2. remove the failing test
  3. update the failing test so that it works again

I'd like to go with proposition 3. However I haven't looked for a solution yet.

@matsduf
Copy link
Contributor

matsduf commented Aug 29, 2022

The issue is not related to this PR but from how the code evolved in Zonemaster-Engine. All new PR will fail in Backend. To solve this we could either:

Yes, I understand that.

2. remove the failing test

3. update the failing test so that it works again

I'd like to go with proposition 3. However I haven't looked for a solution yet.

The API only provides messages with level INFO and higher, and the message has been lowered to debug. I guess that providing a custom profile for the unit test would resolve it. Do you want me to fix that since I caused the problem by the PR in Engine?

@ghost
Copy link
Author

ghost commented Aug 29, 2022

Good idea to update the testing profile. I've created #1051.

@ghost ghost merged commit 523c6e1 into zonemaster:develop Sep 5, 2022
@ghost ghost deleted the api-version-info branch September 5, 2022 10:15
@matsduf matsduf added the V-Minor Versioning: The change gives an update of minor in version. label Nov 27, 2022
@matsduf matsduf added the S-ReleaseTested Status: The PR has been successfully tested in release testing label Dec 17, 2022
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-ReleaseTested Status: The PR has been successfully tested in release testing V-Minor Versioning: The change gives an update of minor in version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant