Page MenuHomeFreeBSD

bridge: Basic test case
ClosedPublic

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

Details

Summary

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

Sponsored by: The FreeBSD Foundation

Diff Detail

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

Event Timeline

melifaro added a subscriber: melifaro.

LGTM. Please see some minor comment inline.

tests/sys/net/if_bridge_test.sh
1 ↗(On Diff #68357)

Maybe it could be #!/usr/bin/env atf-sh to make linter happier? :-)

34 ↗(On Diff #68357)

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 ↗(On Diff #68357)

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
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.
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.