Page MenuHomeFreeBSD

Update rtsock_l3 test after 2fe5a79425c79f7b828acd91da66d97230925fc8
ClosedPublic

Authored by arichardson on Feb 23 2021, 11:52 AM.

Details

Summary

It appears that we now get a different errno. Looking at Jenkins to find
the first commit where this test starting failing indicates that
2fe5a79425c79f7b828acd91da66d97230925fc8 is the most likely cause.

Test Plan

test now passes

Diff Detail

Repository
R10 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

tests/sys/net/routing/test_rtsock_l3.c
398–399

In reality, EEXIST is a previous value from some of the system calls, and the real errno result is 0.
This test should be renamed to something like rtm_get_v4_hostbits_success - didn't have cycles to do so yet.

Update to make them success tests

arichardson added inline comments.
tests/sys/net/routing/test_rtsock_l3.c
452

I'm not sure if this one should actually be a success test, but it does succeed right now.

ping? This test has been failing for the past 208 Jenkins test runs and I'd really like to get CI back to green, so we can actually catch regressions.

ping? This will fix two of the last 5 test failures in CI.

Sorry for the belated reply, will look into it today/tomorrow

Sorry for the belated reply, will look into it today/tomorrow

No problem at all. Thanks for looking.

@melifaro any chance you could have a look at this patch this week?

I've posted D29528 to get CI green again and allow for more time for this patch to be reviewed.

I do not understand enough from the testing infrastructure to help.

I've posted D29528 to get CI green again and allow for more time for this patch to be reviewed.

IMO this is the generally correct approach - once we have a failing test issue identified and a PR open or review in progress it makes sense to XFAIL the test

@melifaro should I split the change to rtm_add_v4_no_rtf_host_failure into a separate review?

@melifaro should I split the change to rtm_add_v4_no_rtf_host_failure into a separate review?

Sorry for taking that long! No, that's totally fine!

This revision is now accepted and ready to land.Apr 21 2021, 8:04 AM

@melifaro should I split the change to rtm_add_v4_no_rtf_host_failure into a separate review?

Sorry for taking that long! No, that's totally fine!

Just to confirm, rtm_add_v4_no_rtf_host_failure is actually supposed to succeed now?

@melifaro should I split the change to rtm_add_v4_no_rtf_host_failure into a separate review?

Sorry for taking that long! No, that's totally fine!

Just to confirm, rtm_add_v4_no_rtf_host_failure is actually supposed to succeed now?

Yes

@melifaro should I split the change to rtm_add_v4_no_rtf_host_failure into a separate review?

Sorry for taking that long! No, that's totally fine!

Just to confirm, rtm_add_v4_no_rtf_host_failure is actually supposed to succeed now?

Yes

Thanks for the review and confirming!

In case of system builded without epair, previously those tests were skipped because the line 'ATF_REQUIRE_KERNEL_MODULE("if_epair");'.
But now it seems they are failing in place of being skipped if epair not builded with message like:

/tests/sys/net/routing/rtsock_config.h:134: Expected errno 17, got 2, in true

In case of system builded without epair, previously those tests were skipped because the line 'ATF_REQUIRE_KERNEL_MODULE("if_epair");'.
But now it seems they are failing in place of being skipped if epair not builded with message like:

/tests/sys/net/routing/rtsock_config.h:134: Expected errno 17, got 2, in true

Sorry about that, will commit a fix shortly.