Page MenuHomeFreeBSD
Differential D4406

Simplify running the FreeBSD Test Suite
AbandonedPublic
Actions

Authored by ngie on Dec 6 2015, 6:35 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Apr 10, 2:59 PM
Unknown Object (File)
Sat, Apr 6, 3:27 PM
Unknown Object (File)
Fri, Mar 29, 12:13 PM
Unknown Object (File)
Feb 29 2024, 7:38 PM
Unknown Object (File)
Feb 26 2024, 2:19 AM
Unknown Object (File)
Dec 20 2023, 12:04 AM
Unknown Object (File)
Dec 16 2023, 9:24 AM
Unknown Object (File)
Dec 16 2023, 9:23 AM
View All Files
Subscribers
eccramer_gmail.com
imp
doceng
tests

Details

Reviewers
emaste
eccramer_gmail.com
jmmv
bdrewery
Group Reviewers
manpages
Summary

Simplify running the FreeBSD Test Suite

Replace make regress (legacy test make target) and make test (incomplete
test make target added with the FreeBSD test suite) with make check as it's
consistent with other open source projects.

make check defaults to running tests from .OBJDIR, but can be overridden
with the CHECKDIR variable.

Add make checkworld target to simplify run the FreeBSD test suite from
TESTSBASE

Document make check and make checkworld in build(7).

Other minor changes:

  • Rename KYUA_PREFIX to LOCALBASE for consistency with other variables.
  • Expose LOCALBASE in Makefile.inc1.
  • Rename intermediate file -- Kyuafile.auto -- to Kyuafile to simplify make check.
  • Remove terse warnings attached to beforetest/aftertest.
  • Add kyua binary check as a dependency of make check in suite.test.mk to make the fact that it's a prerequisite clear and intuitive.

The MFC is contingent on other build related changes being MFCed.

MFC after: 1 month
Relnotes: yes
X-MFC to: stable/10
Sponsored by: EMC / Isilon Storage Division

Test Plan
  • Run make tinderbox (as of writing, in-progress on ref11-amd64.freebsd.org)
  • Run make checkworld
  • Run make check
  • Run make check CHECKDIR=$(make -VTESTSDIR)

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 2343
Build 2358: arc lint + arc unit

Event Timeline

ngie updated this revision to Diff 10808.Dec 6 2015, 6:35 AM
ngie retitled this revision from to Simplify running the FreeBSD Test Suite.
ngie updated this object.
ngie edited the test plan for this revision. (Show Details)
ngie added reviewers: bdrewery, emaste, jmmv.
Herald added a reviewer: manpages. · View Herald TranscriptDec 6 2015, 6:35 AM
Herald added a subscriber: imp. · View Herald Transcript
ngie edited edge metadata.Dec 6 2015, 6:37 AM
ngie added subscribers: tests, doceng.
ngie added inline comments.Dec 6 2015, 6:41 AM
share/mk/atf.test.mk
30 ↗(On Diff #10808)

This should be LOCALBASE too, but I'll handle this in a separate commit.

share/mk/bsd.README
453

"the check target" should be "the realcheck target".

ngie updated this revision to Diff 10810.Dec 6 2015, 7:36 AM
ngie marked an inline comment as done.

Remove some of the unrelated portions of the diff

Herald edited edge metadata. · View Herald TranscriptDec 6 2015, 7:36 AM
ngie updated this object.Dec 6 2015, 7:36 AM
ngie edited edge metadata.
ngie added inline comments.Dec 6 2015, 8:00 AM
share/mk/bsd.README
493–494

The note about it defaulting to TESTSDIR needs to be deleted. This was something I was considering that didn't pan out implementation wise.

share/mk/bsd.test.mk
13

Third-party package prefix (kyua, etc) would probably be more generic.

share/mk/suite.test.mk
57–59

I think .NOPATH breaks tinderbox. Need to remove and retest.

ngie updated this revision to Diff 10829.Dec 6 2015, 9:46 PM
ngie marked 2 inline comments as done.

Fix make tinderbox by only adding Kyuafile to CLEANFILES if KYUAFILE == auto

Also, remove .NOPATH Kyuafile; me removing it passed make tinderbox/make installworld

Herald edited edge metadata. · View Herald TranscriptDec 6 2015, 9:46 PM
eccramer_gmail.com added a subscriber: eccramer_gmail.com.Dec 7 2015, 5:43 PM
eccramer_gmail.com accepted this revision.Dec 16 2015, 1:52 AM
eccramer_gmail.com added a reviewer: eccramer_gmail.com.

I applied this patch in my source tree and ran make check and make checkworld. I've attached the output below.

Everything ran as you would expect. I didn't encounter any problems caused by changes to makefiles.

I did not run make tinderbox as it requires building universe, and that may be a bit much for my machine at home. But at the very least I can sign off on the changes to make check and make checkworld.

makecheck_output_clean817 KBDownload

makecheckworld_output_clean365 KBDownload

This revision is now accepted and ready to land.Dec 16 2015, 1:52 AM
emaste edited edge metadata.Dec 18 2015, 3:26 AM

A few nitpicks but overall I like it. You'll want an UPDATING entry too.

Makefile
124

Suggest leaving regress in for one cycle as

regress:
        @echo 'make regress' has been renamed 'make check'
Makefile.inc1
1348

You can move LOCALBASE= and make this change first in a separate commit

share/man/man7/build.7
119

Probably lower case; I don't think we're trying to establish the "FreeBSD Test Suite" as a proper noun.

share/mk/bsd.README
504

this is not just for kyua, also doxygen as above

share/mk/suite.test.mk
67–75

I'd be fine with these cleanups going in first

84–91

this seems strange to me, I'd expect a if [ ! -x ${LOCALBASE}/bin/kyua ] type test in the intended target instead

ngie marked 5 inline comments as done.Dec 20 2015, 5:29 AM

I appreciate the input :) -- I'll be applying some of the targeted changes to head soon.

Makefile
124

Good suggestion! I'll add a false to the end of the target to make sure that end-users understand that make check is the new target.

share/man/man7/build.7
119

Fair enough. I did it to be consistent with the wiki page, but I agree.

share/mk/bsd.README
504

Right, but this section's for bsd.test.mk.

share/mk/suite.test.mk
84–91

Yeah, you're right. There might be some unintended side effects of doing it this way, so I'll do an inline if statement instead.

ngie updated this revision to Diff 11483.Dec 20 2015, 6:21 AM
ngie edited edge metadata.
ngie marked 7 inline comments as done.

Update diff based on feedback received by emaste

This revision now requires review to proceed.Dec 20 2015, 6:21 AM
Herald edited edge metadata. · View Herald TranscriptDec 20 2015, 6:21 AM
ngie updated this revision to Diff 11487.Dec 20 2015, 6:31 AM
ngie edited edge metadata.

More diff reduction after further cleanup

Herald edited edge metadata. · View Herald TranscriptDec 20 2015, 6:31 AM
ngie updated this revision to Diff 11488.Dec 20 2015, 6:32 AM
ngie edited edge metadata.

Fix grammar in the make regress "pointer target"

Herald edited edge metadata. · View Herald TranscriptDec 20 2015, 6:32 AM
ngie added a comment.Jan 12 2016, 10:53 PM

Ping?

ngie marked 3 inline comments as done.Jan 13 2016, 12:27 AM
bdrewery accepted this revision.Jan 18 2016, 5:01 PM
bdrewery edited edge metadata.

Seems odd to still have TESTSDIR rather than CHECKSDIR. It's also borderline against POLA to rename a feature that has been released already for the sake of "other projects", especially without some kind of test: target to warn people or be backwards-compat.

This revision is now accepted and ready to land.Jan 18 2016, 5:01 PM
emaste added a comment.Jan 18 2016, 5:22 PM
In D4406#105579, @bdrewery wrote:

It's also borderline against POLA to rename a feature that has been released already for the sake of "other projects"

It seems make check is standard in the autotools world but make test is also widely used by various projects (a quick search turned up well known examples like Samba, DPDK, and OpenSSL). @bdrewery claims make test was never really completed, and make regress does seem to be a rather obscure outlier, so I don't think there's a significant POLA concern here.

especially without some kind of test: target to warn people or be backwards-compat.

I agree here though - make test should at least emit an error message informing the user to use make check instead.

ngie added a comment.Jan 30 2016, 7:11 PM
In D4406#105588, @emaste wrote:
In D4406#105579, @bdrewery wrote:

It's also borderline against POLA to rename a feature that has been released already for the sake of "other projects"

It seems make check is standard in the autotools world but make test is also widely used by various projects (a quick search turned up well known examples like Samba, DPDK, and OpenSSL). @bdrewery claims make test was never really completed, and make regress does seem to be a rather obscure outlier, so I don't think there's a significant POLA concern here.

especially without some kind of test: target to warn people or be backwards-compat.

I agree here though - make test should at least emit an error message informing the user to use make check instead.

This will cause grief for some areas of the tree. Example:

$ cd /usr/src/svn/bin/test
$ make obj; make test
===> tests (obj)
/usr/obj/usr/src/svn/bin/test/tests created for /usr/src/svn/bin/test/tests
cc -O2 -pipe -O2 -pipe   -g -std=gnu99 -fstack-protector-strong -Wsystem-headers -Werror -Wall -Wno-format-y2k -W -Wno-unused-parameter -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Wreturn-type -Wcast-qual -Wwrite-strings -Wswitch -Wshadow -Wunused-parameter -Wcast-align -Wchar-subscripts -Winline -Wnested-externs -Wredundant-decls -Wold-style-definition -Wno-pointer-sign -Wmissing-variable-declarations -Wthread-safety -Wno-empty-body -Wno-string-plus-int -Wno-unused-const-variable  -Qunused-arguments -c /usr/src/svn/bin/test/test.c -o test.o
cc -O2 -pipe -O2 -pipe -g -std=gnu99 -fstack-protector-strong -Wsystem-headers -Werror -Wall -Wno-format-y2k -W -Wno-unused-parameter -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Wreturn-type -Wcast-qual -Wwrite-strings -Wswitch -Wshadow -Wunused-parameter -Wcast-align -Wchar-subscripts -Winline -Wnested-externs -Wredundant-decls -Wold-style-definition -Wno-pointer-sign -Wmissing-variable-declarations -Wthread-safety -Wno-empty-body -Wno-string-plus-int -Wno-unused-const-variable -Qunused-arguments  -o test.full test.o  
objcopy --only-keep-debug test.full test.debug
objcopy --strip-debug --add-gnu-debuglink=test.debug  test.full test$ make obj; make test
===> tests (obj)
/usr/obj/usr/src/svn/bin/test/tests created for /usr/src/svn/bin/test/tests
cc -O2 -pipe -O2 -pipe   -g -std=gnu99 -fstack-protector-strong -Wsystem-headers -Werror -Wall -Wno-format-y2k -W -Wno-unused-parameter -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Wreturn-type -Wcast-qual -Wwrite-strings -Wswitch -Wshadow -Wunused-parameter -Wcast-align -Wchar-subscripts -Winline -Wnested-externs -Wredundant-decls -Wold-style-definition -Wno-pointer-sign -Wmissing-variable-declarations -Wthread-safety -Wno-empty-body -Wno-string-plus-int -Wno-unused-const-variable  -Qunused-arguments -c /usr/src/svn/bin/test/test.c -o test.o
cc -O2 -pipe -O2 -pipe -g -std=gnu99 -fstack-protector-strong -Wsystem-headers -Werror -Wall -Wno-format-y2k -W -Wno-unused-parameter -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Wreturn-type -Wcast-qual -Wwrite-strings -Wswitch -Wshadow -Wunused-parameter -Wcast-align -Wchar-subscripts -Winline -Wnested-externs -Wredundant-decls -Wold-style-definition -Wno-pointer-sign -Wmissing-variable-declarations -Wthread-safety -Wno-empty-body -Wno-string-plus-int -Wno-unused-const-variable -Qunused-arguments  -o test.full test.o  
objcopy --only-keep-debug test.full test.debug
objcopy --strip-debug --add-gnu-debuglink=test.debug  test.full test
$

This is one of the prime reasons why I'm moving away from make test. It was horribly broken in cases in the base system :(...

ngie updated this revision to Diff 12881.Jan 30 2016, 7:23 PM
ngie edited edge metadata.

Update after recent changes to bsd.subdir.mk for -exp run

This revision now requires review to proceed.Jan 30 2016, 7:23 PM
Herald edited edge metadata. · View Herald TranscriptJan 30 2016, 7:23 PM
ngie added a comment.Feb 4 2016, 9:16 AM

The exp- run finished successfully: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=205718 -- are there any additional comments about the proposed change?

ngie abandoned this revision.Feb 7 2016, 6:43 PM

Committed in rS295380.

Revision Contents
Changeset List

PathSize
11 lines
15 lines
share/
man/
man7/
13 lines
mk/
21 lines
4 lines
6 lines
19 lines
53 lines

Diff 12881

View Options

Makefile

Loading...
View Options

Makefile.inc1

Loading...
View Options

share/man/man7/build.7

Loading...
View Options

share/mk/bsd.README

Loading...
View Options

share/mk/bsd.subdir.mk

Loading...
View Options

share/mk/bsd.sys.mk

Loading...
View Options

share/mk/bsd.test.mk

Loading...
View Options

share/mk/suite.test.mk

Loading...