Details

Reviewers

Group Reviewers

Commits

rGb4b99d2333fa: test/libalias: Tests for instantiation and outgoing NAT
rGfb70e02fafc1: test/libalias: Tests for instantiation and outgoing NAT
rGcfeeb57166d9: test/libalias: Tests for instantiation and outgoing NAT
rG7fd8baee756e: test/libalias: Tests for instantiation

Summary

In order to modify libalias for performance, the existing functionality must not change.

This review is only a starter of the larger set of functional tests which needs to be upheld during the upcoming development. More test cases will come in further reviews.

I'll keep the review small enough to be readable by an external person and hence the commits small enough to not disturb the developer community.

Test Plan

$ make test
1_instance:1_singleinit  ->  passed  [0.003s]
1_instance:2_destroynull  ->  expected_failure: Code expects valid pointer.  [0.013s]
1_instance:3_multiinit  ->  passed  [0.005s]
1_instance:4_multiinstance  ->  passed  [0.038s]
4/4 passed (0 failed)

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 39241
Build 36130: arc lint + arc unit

Event Timeline

donner created this revision.May 16 2021, 9:41 PM

Herald added subscribers: Contributor Reviews (src), melifaro, imp. · View Herald TranscriptMay 16 2021, 9:42 PM

donner requested review of this revision.May 16 2021, 9:42 PM

Harbormaster completed remote builds in B39233: Diff 89321.May 16 2021, 9:42 PM

Start as simple as possible and fail ...

[/usr/tests/sys/netinet/libalias]$ kyua list
aliasdb_test:__test_cases_list__

But calling directly works:

$ /usr/tests/sys/netinet/libalias/aliasdb_test -l
Content-Type: application/X-atf-tp; version="1"

ident: init
has.cleanup: true
desc: Create an instance

and

$ /usr/tests/sys/netinet/libalias/aliasdb_test init
aliasdb_test: WARNING: Running test cases outside of kyua(1) is unsupported
aliasdb_test: WARNING: No isolation nor timeout control is being applied; you may get unexpected failures; see atf-test-case(4)
(null)
(null)
(null)
(null)
(null)
(null)
passed

Ah, btw: Is there a possibilty to run it as a user without installing it?

lwhsu added a subscriber: lwhsu.May 17 2021, 3:49 AM

Simplify the test

Harbormaster completed remote builds in B39235: Diff 89334.May 17 2021, 6:49 AM

Remove unnecessary static

Harbormaster completed remote builds in B39236: Diff 89335.May 17 2021, 6:51 AM

Ah, kyua report shows:

aliasdb_test:__test_cases_list__  ->  broken: Unknown test case metadata property 'desc'  [0.001s]

Fix meta data. => Now it works.

Harbormaster completed remote builds in B39237: Diff 89336.May 17 2021, 6:56 AM

Add test to recreate an instance multiple time.

Harbormaster completed remote builds in B39238: Diff 89339.May 17 2021, 7:27 AM

Test to create many instances and remove them in random order.

Harbormaster completed remote builds in B39239: Diff 89340.May 17 2021, 8:36 AM

Add a test for a known API violation.

Harbormaster completed remote builds in B39240: Diff 89341.May 17 2021, 9:38 AM

Make the tests work directly on the intermediate builds.

Harbormaster completed remote builds in B39241: Diff 89343.May 17 2021, 9:54 AM

donner added a reviewer: network.May 17 2021, 11:35 AM

I think you're also missing a makefile hookup somewhere. These tests may want to live in /usr/src/tests/sys/netinet/libalias instead, but I don't hold strong views on that.

sys/netinet/libalias/tests/Makefile
14	That probably wants ${.OBJDIR}
sys/netinet/libalias/tests/aliasdb_test.c
8	I wonder if it's worth printing the random seed somewhere, or even using a fixed seed. If this triggers a bug it may end up being nondeterministic (and thus difficult to debug).

Use OBJDIR
Make random reproducable.
Inspect kyua reports

Harbormaster completed remote builds in B39246: Diff 89359.May 17 2021, 3:47 PM

donner marked an inline comment as done.May 17 2021, 3:47 PM

donner marked an inline comment as done.

Replace preprocessor constants by compiler generated values.

Harbormaster completed remote builds in B39247: Diff 89360.May 17 2021, 4:04 PM

Renumber the test cases and rename the file according to scope

Harbormaster completed remote builds in B39252: Diff 89367.May 17 2021, 6:38 PM

donner retitled this revision from libalias: Create a test suite to libalias: Initial test suite.May 17 2021, 6:45 PM

donner edited the summary of this revision. (Show Details)

donner edited the test plan for this revision. (Show Details)

donner added a reviewer: kp.

In D30307#680794, @kp wrote:

I think you're also missing a makefile hookup somewhere. These tests may want to live in /usr/src/tests/sys/netinet/libalias instead, but I don't hold strong views on that.

libalias is primary sourced in sys/netinet/libalias. The userspace variant is symlinked into lib/libalias.
That's why I'd keep the tests at the real source.

Any objections?

In D30307#680887, @donner wrote:

Any objections?

No, my views on the subject are very weak. I would like to see this test hooked into the build system though, so it gets built and installed as part of build/install world.