bridge: Basic test case
ClosedPublic
Actions

Authored by kp on Feb 15 2020, 5:29 PM.

Details

Reviewers

emaste
philip
melifaro

Group Reviewers

network

Commits

rG791f370cb5d6: bridge: Basic test case
rS357998: bridge: Basic test case

Summary

Very basic bridge test: Set up two jails and test that they can pass
traffic over the bridge.

Diff Detail

Lint

Lint Warnings

Severity	Location	Code	Message
Warning	tests/sys/net/if_bridge_test.sh:	CHMOD1	Invalid Executable

Unit

No Test Coverage

Build Status

Buildable 29402
Build 27288: arc lint + arc unit

Event Timeline

kp created this revision.Feb 15 2020, 5:29 PM

Herald added a subscriber: ae. · View Herald TranscriptFeb 15 2020, 5:29 PM

Harbormaster completed remote builds in B29402: Diff 68357.Feb 15 2020, 5:29 PM

LGTM. Please see some minor comment inline.

tests/sys/net/if_bridge_test.sh
1	Maybe it could be `#!/usr/bin/env atf-sh` to make linter happier? :-)
34	Would it be possible to name it something like 'bridge_transmit_ipv4_unicast' so once we add more tests (IPv4/IPv6, unicast, multicast, etc.) it would be consistent with others?

This revision is now accepted and ready to land.Feb 15 2020, 5:53 PM

This looks good to me. I had one minor style observation in addition to the ones @melifaro caught.

tests/sys/net/Makefile
13	While you're here, these could be alphabetised.

sort test list in Makefile
rename test case

I did not add the shebang, because those are added during the installation
step. If we add one to the source file we'll end up with two shebang lines in
the installed file.

This revision now requires review to proceed.Feb 16 2020, 10:41 AM

Harbormaster completed remote builds in B29422: Diff 68400.Feb 16 2020, 10:41 AM

melifaro accepted this revision.Feb 16 2020, 10:43 AM

This revision is now accepted and ready to land.Feb 16 2020, 10:43 AM

This revision was landed with ongoing or failed builds.Feb 16 2020, 1:16 PM

Closed by commit rS357998: bridge: Basic test case (authored by kp). · Explain Why

This revision was automatically updated to reflect the committed changes.

kp added a commit: rS357998: bridge: Basic test case.

Herald added a subscriber: imp. · View Herald TranscriptFeb 16 2020, 1:16 PM

In D23697#520613, @kp wrote:

sort test list in Makefile

rename test case

Thank you! This looks good now.

I did not add the shebang, because those are added during the installation
step. If we add one to the source file we'll end up with two shebang lines in
the installed file.

That makes sense. I don't think we need to worry about running tests from the source directory. The fact that the absent shebang makes Phabricator cry is unfortunate but not something we should lose sleep over.

kp added a commit: rG791f370cb5d6: bridge: Basic test case.Mar 2 2021, 3:19 PM