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

tests/gnrc_dhcpv6_client: fix expected output #14524

Merged
merged 1 commit into from
Jul 15, 2020

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Jul 15, 2020

Contribution description

The output of the shell command nib prefix show was changed in #12776 by adding a space between the number of seconds and the unit. Due to this the test tests/gnrc_dhcpv6_client now fails as it expects the output to be without spaces.

As the new riotctrl_shell "accidentally" accounted for that "error", I decided to rather fix the test than reverting the addition of the whitespace. There is no other test in master that checks this output.

Testing procedure

The test requires kea to be installed. It is available in most Linux distributions as a package, but many don't create the status directory of the server:

sudo mkdir -p /var/kea/

Then run on a board of your choice (that supports ethos):

make -C tests/gnrc_dhcpv6_client flash
sudo make -C tests/gnrc_dhcpv6_client test

Issues/PRs references

Fixes a "regression" introduced in #12776

@miri64 miri64 added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: network Area: Networking Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs labels Jul 15, 2020
@miri64 miri64 added this to the Release 2020.07 milestone Jul 15, 2020
@miri64 miri64 requested review from maribu and fjmolinas July 15, 2020 14:34
@miri64 miri64 requested a review from benpicco July 15, 2020 17:43
Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

Patch works on native, but the test is buggy:

  • on native it is required to run dist/tools/tapsetup/tapsetup before the test works. This is not documented.
  • on a board (e.g. samr21-xpro with ethos) the test does not work at all as kea-dhcp6.conf hard-codes tapbr0.

@miri64
Copy link
Member Author

miri64 commented Jul 15, 2020

* on `native` it is required to run `dist/tools/tapsetup/tapsetup` before the test works. This is not documented.

Where should this be documented if not in the README (where it is documented) ;-)

@miri64
Copy link
Member Author

miri64 commented Jul 15, 2020

* on a board (e.g. `samr21-xpro` with `ethos`) the test does not work at all as `kea-dhcp6.conf` hard-codes `tapbr0`.

Also documented in the test's README...

@miri64 miri64 merged commit 7150c7a into master Jul 15, 2020
@benpicco benpicco deleted the tests/fix/gnrc_dhcpv6_client-pl-output branch July 15, 2020 18:18
@miri64
Copy link
Member Author

miri64 commented Jul 15, 2020

Thanks for testing and reviewing :-)

@miri64
Copy link
Member Author

miri64 commented Jul 15, 2020

Backport provided in #14528

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants