Page MenuHomeFreeBSD

Simplify running the FreeBSD Test Suite
AbandonedPublic

Authored by ngie on Dec 6 2015, 6:35 AM.

Details

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 OK
Unit
No Unit Test Coverage
Build Status
Buildable 2343
Build 2358: arc lint + arc unit

Event Timeline

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.
ngie added subscribers: tests, doceng.
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
452

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

ngie marked an inline comment as done.

Remove some of the unrelated portions of the diff

ngie edited edge metadata.
share/mk/bsd.README
492–493

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
12

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

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.

This revision is now accepted and ready to land.Dec 16 2015, 1:52 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
118

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

share/mk/bsd.README
503

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

83–90

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
118

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

share/mk/bsd.README
503

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

share/mk/suite.test.mk
83–90

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 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
ngie edited edge metadata.

More diff reduction after further cleanup

ngie edited edge metadata.

Fix grammar in the make regress "pointer target"

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

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.

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

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