Page MenuHomeFreeBSD

ping6: add a basic test of ping6's functionality
ClosedPublic

Authored by sucanjan_gmail.com on Aug 16 2019, 4:20 PM.

Details

Summary

Add a basic test of ping6's functionality

Submitted by: Ján Sučan <sucanjan@gmail.com>
Sponsored by: Google, inc. (Google Summer of Code 2019)

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

asomers requested changes to this revision.Aug 17 2019, 2:30 PM

This test will fail if IPv6 is not configured at runtime. It needs some kind of check for that.

This revision now requires changes to proceed.Aug 17 2019, 2:30 PM

Could I use getaddrinfo command for that? Like this:

if getaddrinfo -f inet6 localhost 1>/dev/null 2>&1; then
    atf_add_test_case ping6_c3

Could I use getaddrinfo command for that? Like this:

if getaddrinfo -f inet6 localhost 1>/dev/null 2>&1; then
    atf_add_test_case ping6_c3

Yes, you can use getaddrinfo. However, don't use it in atf_init_test_cases. Instead, use it at the top of the test body (possibly in a dedicated function). If it fails, then call atf_skip.

Use a single tab width in the ATF test script.

Skip the test case if IPv6 is not configured.

This looks mostly good, but I have two concerns:

  1. Should you filter out hop limit the same way you filtered out ttl from the ping tests?
  2. The test case takes more than 2 seconds to run. That doesn't sound like much, but it adds up over more than 7000 tests. I suggest reducing the packet count to 1. If you also want to test > 1 packet, then you can add a second test case that requires root privileges and uses -i to reduce the wait.

Yes, the TTL will be filtered out. Both ping and ping6 tests will be sending only one packet.

I think it would be good to add -t 1 (timeout in seconds) for one packet just to be sure that the test case will not wait for the reply forever (I don't know if ATF has some timeout).

Yes, the TTL will be filtered out. Both ping and ping6 tests will be sending only one packet.
I think it would be good to add -t 1 (timeout in seconds) for one packet just to be sure that the test case will not wait for the reply forever (I don't know if ATF has some timeout).

Good idea. ATF's default timeout is much too long for this test (5 minutes).

Rebase.
Send only one packet.
Filter out TTL value.
Specify packet size on the command line.
Add -X 1 option to reduce time of waiting for the reply.
Add -u option to diff. This makes the result file more readable in case of an error.

asomers requested changes to this revision.Thu, Aug 22, 2:21 PM

Please add a copyright header with a $FreeBSD$ tag to the top of ping6_test.sh. Also, add an entry to etc/mtree/BSD.tests.dist.

This revision now requires changes to proceed.Thu, Aug 22, 2:21 PM

The copyright header and the tag are going to be added.

There is already an entry for ping6 test directory in etc/mtree/BSD.tests.dist in this diff. Could you please let me know whether I understand the requirement correctly?

Add a copyright header with a $FreeBSD$ tag to ping6_test.sh.

asomers accepted this revision.Thu, Aug 22, 3:05 PM

Uhh, the requirement would be that I need to read the entire review, not just the most recent commit.

This revision is now accepted and ready to land.Thu, Aug 22, 3:05 PM
This revision was automatically updated to reflect the committed changes.