Project Tags

None

Referenced Files

	F177559187: D61066.1794718842.diff
	Sat, Nov 14, 5:00 AM

	F177556829: D61066.1794713515.diff
	Sat, Nov 14, 3:31 AM

	F177552814: D61066.1794699469.diff
	Fri, Nov 13, 11:37 PM

	F177552767: D61066.1794699384.diff
	Fri, Nov 13, 11:36 PM

Subscribers

emaste

imp

jonathan

Details

Reviewers

markj

Commits

rG58e5c5cac1e7: tests/sys/capsicum: restore test coverage with root user
rGdda15b669d02: tests/sys/capsicum: restore test coverage with root user

Summary

This change builds capsicum_test and installs/runs it as an unprivileged
user ("tests" in vanilla FreeBSD) and root.

This required adjusting logic in the Makefile in order to build two
binaries with the same sources and running them as separate binaries.

Moreover, in order to restore coverage and ensure that some tests which
require other_uid (typically specified via -u on the CLI with the
test program) don't fail, become the "tests" user when run as root. This
technically doesn't honor unprivileged_user property in kyua.conf: the
property needs to be passed via a command-line parameter to the test
program.

MFC after: 1 week
Reported by: markj
Fixes: 3fb03fb35b20 ("[tests] switch GoogleTest tests ...")

Test Plan

cosmic-crisp-16-current% sudo kyua test -k /usr/tests/sys/capsicum/Kyuafile
...
capsicum-test_unprivileged:Sctp.Socket  ->  skipped: socket(..., IPPROTO_SCTP) -> EPROTONOSUPPORT

  [0.007s]
capsicum-test_unprivileged:Select.LotsOFileDescriptorsForked  ->  passed  [0.019s]
capsicum-test_unprivileged:Socket.TCP  ->  passed  [0.008s]
capsicum-test_unprivileged:Socket.UDP  ->  passed  [0.008s]
capsicum-test_unprivileged:Socket.UnixDomain  ->  passed  [0.010s]
capsicum-test_unprivileged:Sysctl.Capability  ->  passed  [0.007s]

Results file id is usr_tests_sys_capsicum.20261030-230922-165891
Results saved to /root/.kyua/store/results.usr_tests_sys_capsicum.20261030-230922-165891.db

213/220 passed (0 broken, 0 failed, 7 skipped)

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 78585
Build 75468: arc lint + arc unit

Event Timeline

ngie created this revision.Fri, Oct 30, 11:07 PM

Herald added subscribers: jonathan, imp. · View Herald TranscriptFri, Oct 30, 11:07 PM

ngie requested review of this revision.Fri, Oct 30, 11:07 PM

Harbormaster completed remote builds in B78585: Diff 191075.Fri, Oct 30, 11:07 PM

ngie edited the test plan for this revision. (Show Details)Fri, Oct 30, 11:09 PM

ngie added a reviewer: markj.

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

ngie added a subscriber: emaste.

markj added inline comments.Sat, Oct 31, 4:13 PM

tests/sys/capsicum/Makefile
17	Shouldn't this be scoped to the capsicum tests, i.e., `WARNS.capsicum-test_privileged?=3 ...`?
tests/sys/capsicum/capability-fd.cc
1265	These tags aren't needed, this isn't contrib code (anymore).
tests/sys/capsicum/capsicum-test.h
47	Why abort? Shouldn't this skip the test or use EXPECT_NE or something?
tests/sys/capsicum/procdesc.cc
568	This setup should be done in main(), not in individual tests.

ngie added inline comments.Sat, Oct 31, 4:57 PM

tests/sys/capsicum/Makefile
17	Shouldn't this be scoped to the capsicum tests, i.e., `WARNS.capsicum-test_privileged?=3 ...`? I tried doing that and it didn't work - I'll try it again.
tests/sys/capsicum/capsicum-test.h
47	Why abort? Shouldn't this skip the test or use EXPECT_NE or something? I tried using `ASSERT_NE` (what it should be) and `EXPECT_NE`, but the macros return `const char *` under the covers instead of raising an exception, which obviously doesn't work with functions that return `uid_t`. I'll just convert the function into a do-while(0) wrapped macro so it can be used in the tests with `ASSERT_NE`.
tests/sys/capsicum/procdesc.cc
568	This setup should be done in main(), not in individual tests. Sure.

ngie added inline comments.Sat, Oct 31, 4:59 PM

tests/sys/capsicum/capability-fd.cc
1265	These tags aren't needed, this isn't contrib code (anymore). shrug yeah.. I should have just wrapped this with a comment to the bug.

ngie added inline comments.Sat, Oct 31, 6:01 PM

tests/sys/capsicum/Makefile
17	Shouldn't this be scoped to the capsicum tests, i.e., `WARNS.capsicum-test_privileged?=3 ...`? I tried doing that and it didn't work - I'll try it again. That didn't work -_-... it's trying to compile the tests with `WARNS?= 6` (I suspect it's a clash between bsd.sys.mk and how bsd.progs.mk handles things).

ngie mentioned this in D61080: capsicum-test: make the code WARNS=6 clean.Sat, Oct 31, 7:50 PM

ngie added a parent revision: D61080: capsicum-test: make the code WARNS=6 clean.Sat, Oct 31, 7:53 PM

Shuffle "tests" user resolving off to main(..).

Change comments to suit change.

Remove WARNS=3 since the code is WARNS=6 clean after D61080.

Harbormaster completed remote builds in B78616: Diff 191138.Sat, Oct 31, 7:59 PM

ngie marked an inline comment as done.Sat, Oct 31, 8:01 PM

ngie added inline comments.

tests/sys/capsicum/Makefile
17	Shouldn't this be scoped to the capsicum tests, i.e., `WARNS.capsicum-test_privileged?=3 ...`? I tried doing that and it didn't work - I'll try it again. That didn't work -_-... it's trying to compile the tests with `WARNS?= 6` (I suspect it's a clash between bsd.sys.mk and how bsd.progs.mk handles things). Resolving `WARNS` issues in D61080.
59	I should technically move this to the other revision.