mac_bsdextended: ATFify the tests
ClosedPublic
Actions

Authored by asomers on Apr 2 2020, 4:45 AM.

Details

Reviewers

ngie
lwhsu

Commits

rS361126: MFC r360339, r360567
rS360339: mac_bsdextended: ATFify the tests

Summary

mac_bsdextended: ATFify the tests

This is a precondition to fixing bug 244229

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

asomers created this revision.Apr 2 2020, 4:45 AM

Herald added a subscriber: imp. · View Herald TranscriptApr 2 2020, 4:45 AM

Harbormaster completed remote builds in B30237: Diff 70107.Apr 2 2020, 4:46 AM

This is a precondition to fixing bug 244229

I understand why this was a precondition to fixing bug 244229 after reading the bug, but without that context, I wouldn't know why. Could you please clarify that in the commit message?

tests/sys/mac/bsdextended/Makefile
6 ↗	(On Diff #70107)	nit: why not sort `matches_test` above `ugidfw_test`, since ATF < TAP, sorting-wise?
tests/sys/mac/bsdextended/matches_test.sh
70 ↗	(On Diff #70107)	Why isn't this filename parameterized at the global level?
316 ↗	(On Diff #70107)	Why not set this property in the Makefile?
375 ↗	(On Diff #70107)	Wouldn't it be a good idea here (and elsewhere) to check the return code from ugidfw, etc?
410 ↗	(On Diff #70107)	Why not use `atf_check` with an inverted exit code check?

ngie added inline comments.Apr 2 2020, 5:11 AM

tests/sys/mac/bsdextended/matches_test.sh
14 ↗	(On Diff #70107)	Where is this called in the new code?

asomers marked 3 inline comments as done.Apr 3 2020, 1:10 AM

asomers added inline comments.

tests/sys/mac/bsdextended/matches_test.sh
70 ↗	(On Diff #70107)	Because it's the result of `mktemp`. Though, now that we're using ATF, it doesn't have to be. I'll change it.
316 ↗	(On Diff #70107)	I generally prefer to set those things in the test file because it's more granular. It's done per testcase here, whereas it must be per test program if done in the Makefile. Do you think I should change it?
375 ↗	(On Diff #70107)	Yeah, probably.
410 ↗	(On Diff #70107)	Sure. It will remove the failure descriptions, but those aren't very useful anyway.

mac_bsdextended: ATFify the tests

The new tests have more complete setup and cleanup, are more granular, and
correctly annotate expected failures and skipped tests. A follow-up commit
will resolve a conflict with the fusefs tests (bug 244229)

Harbormaster completed remote builds in B30253: Diff 70137.Apr 3 2020, 1:10 AM

ping @ngie . Does it look better now?

ping @ngie @lwhsu . Are you satisfied with the latest changes?

This revision was not accepted when it landed; it landed in state Needs Review.Apr 26 2020, 3:51 PM

Closed by commit rS360339: mac_bsdextended: ATFify the tests (authored by asomers). · Explain Why

This revision was automatically updated to reflect the committed changes.

asomers added a commit: rS360339: mac_bsdextended: ATFify the tests.

asomers added a commit: rS361126: MFC r360339, r360567.May 17 2020, 2:36 AM