Page MenuHomeFreeBSD

Add limited sandbox capability to "make check"
ClosedPublic

Authored by ngie on Aug 7 2017, 12:48 AM.

Details

Summary

Add limited sandbox capability to "make check"

Rationale

r295380 introduced "make check" and consolidated means for running
test code in an attempt to simplify running tests. One could either
install files/libraries/programs and run "make check", or run "make check"
with an explicit CHECKDIR, e.g., make check CHECKDIR=$(make -V.OBJDIR)`.

One criticism that was received is that "make check" should be run with
the intent of making dev->test->commit easier, which means that the target
audience's workflow should be developers. One developer pattern available
in other opensource projects is to run test code from a developer sandbox,
instead of installing to a system.

Method

This approach is slightly different from the standard approach, in the sense
that it builds and installs into a deterministic directory under .OBJDIR (as I call it,
the "sandbox"), then runs "make check" against that. In the event the test
run is successful, the deterministic directory is removed to save space.

Approach

bsd.lib.mk, bsd.prog.mk:

To support this functionality, a new variable HAS_TESTS is being added.

HAS_TESTS enables appropriate behavior with bsd.lib.mk and bsd.prog.mk, as
follows:

  • Add "make check" as an available target from the directory.
  • Pass down appropriate variables via ${TESTS_ENV}, i.e., ${TESTS_LD_LIBRARY_PATH} and ${TESTS_PATH}.

One should add "HAS_TESTS" to directories containing tests in them, e.g. from
bin/sh/Makefile,

HAS_TESTS=
SUBDIR.${MK_TESTS}+= tests

HAS_TESTS doesn't automatically add the tests subdirectory for flexibility
reasons.

bsd.opts.mk, src.opts.mk:

  • The knob ${MK_MAKE_CHECK_USE_SANDBOX} has been added, both to explicitly direct (internally) when to set a deterministic ${DESTDIR} and to also allow users to disable this behavior globally, i.e., via src.conf.
  • MK_TESTS has been promoted from src.opts.mk to bsd.opts.mk to leverage syntactic sugar for having MK_TESTS be a dependency for MK_MAKE_CHECK_USE_SANDBOX, but to also ensure that src.opts.mk isn't required to use suite.test.mk (which is a dependency of bsd.test.mk).

suite.test.mk:

  • beforecheck behavior (when MK_MAKE_CHECK_USE_SANDBOX is enabled) is modified from a no-op to:
    • Build.
    • Run "make hierarchy" on the sandbox dir.
    • Install the tests/files to the sandbox dir.
  • aftercheck behavior (when MK_MAKE_CHECK_USE_SANDBOX is enabled) is modified from a no-op to:
    • Remove the sandbox dir.

Again, because the dependency order set in bsd.test.mk is
beforecheck -> check -> aftercheck, "make check" will not be run unless
"beforecheck" completes successfully, and "aftercheck" will not be run unless
"beforecheck" and "check" complete successfully.

Caveats

  • This target must either be run with MK_INSTALL_AS_USER or as root. Otherwise it will fail when running "make install" as the default user/group for many makefiles when calling INSTALL is root/wheel.
  • This target must be run from a suitable top-level directory. For example, running tests from tests/sys/fs/tmpfs won't work, but tests/sys/fs will, because tests/sys/fs/tmpfs relies on files installed by tests/sys/fs.
  • Running MK_INSTALL_AS_USER may introduce determinism issues. However, using it could identify deficiences in tests in terms of needing to be run as root, which are not properly articulated in the test requirements.
  • The doesn't negate the need for running "make installworld" and "make checkworld", etc. Again, this just is intended to simplify the dev->test->commit workflow.

Cleanup done

  • CHECKDIR is removed; one can use "MK_MAKE_CHECK_USE_SANDBOX=no" to enable "legacy" (r295380) behavior.

MFC after: 2 months
Relnotes: yes (CHECKDIR removed; "make check" behavior changed)
Requested by: jhb

Test Plan
  • Buildworld/installworld
  • "make check" from bin/sh and tests/

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ngie added a subscriber: tests.

I suggest looking at one of the changed Makefiles, e.g., bin/sh/Makefile, then the changes in share/. If desired, I could try splitting up the review/commit as 2 separate items.

Limit CR to files in share/mk

This revision was automatically updated to reflect the committed changes.