Paths

Table of Contentst

Update rtsock_l3 test after 2fe5a79425c79f7b828acd91da66d97230925fc8
ClosedPublic
Actions

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

Details

Reviewers

melifaro
donner

Group Reviewers

network
tests

Commits

rGef4cbfa6ea15: Update rtsock_l3 test after 2fe5a79425c79f7b828acd91da66d97230925fc8
rG67f2f67fc8ce: Update rtsock_l3 test after 2fe5a79425c79f7b828acd91da66d97230925fc8

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

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 38587
Build 35476: arc lint + arc unit

Event Timeline

arichardson created this revision.Feb 23 2021, 11:52 AM

Herald added subscribers: ae, imp. · View Herald TranscriptFeb 23 2021, 11:52 AM

arichardson requested review of this revision.Feb 23 2021, 11:52 AM

Harbormaster completed remote builds in B37300: Diff 84529.Feb 23 2021, 11:53 AM

melifaro added inline comments.Feb 24 2021, 11:24 PM

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

Harbormaster completed remote builds in B37509: Diff 84994.Mar 2 2021, 7:23 PM

arichardson marked an inline comment as done.Mar 4 2021, 9:48 AM

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

In D28886#656319, @melifaro wrote:

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

No problem at all. Thanks for looking.

Weekly ping.

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

arichardson added a reviewer: tests.Mar 30 2021, 11:25 AM

arichardson mentioned this in D29528: tests/sys/net/routing: XFAIL the two failing tests.Apr 1 2021, 8:58 AM

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

arichardson mentioned this in rG83532eb68cd0: tests/sys/net/routing: XFAIL the two failing tests.Apr 7 2021, 10:36 AM

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

emaste added a subscriber: emaste.Apr 7 2021, 2:05 PM

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

arichardson mentioned this in rG676010ab86f5: tests/sys/net/routing: XFAIL the two failing tests.Apr 10 2021, 1:31 PM

Rebase after D29528

Harbormaster completed remote builds in B38587: Diff 87477.Apr 15 2021, 10:24 AM

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

In D28886#669858, @arichardson wrote:

@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!

melifaro accepted this revision.Apr 21 2021, 8:04 AM

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

In D28886#670565, @melifaro wrote:

In D28886#669858, @arichardson wrote:

@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?

In D28886#670581, @arichardson wrote:

In D28886#670565, @melifaro wrote:

In D28886#669858, @arichardson wrote:

@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

In D28886#670585, @melifaro wrote:

In D28886#670581, @arichardson wrote:

In D28886#670565, @melifaro wrote:

In D28886#669858, @arichardson wrote:

@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!

Closed by commit rG67f2f67fc8ce: Update rtsock_l3 test after 2fe5a79425c79f7b828acd91da66d97230925fc8 (authored by arichardson). · Explain WhyApr 21 2021, 10:01 AM

This revision was automatically updated to reflect the committed changes.

arichardson added a commit: rG67f2f67fc8ce: Update rtsock_l3 test after 2fe5a79425c79f7b828acd91da66d97230925fc8.

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 D28886#671757, @olivier wrote:
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.

melifaro added a commit: rGef4cbfa6ea15: Update rtsock_l3 test after 2fe5a79425c79f7b828acd91da66d97230925fc8.Jan 13 2023, 9:27 PM

Herald added a subscriber: glebius. · View Herald TranscriptJan 13 2023, 9:27 PM