Page MenuHomeFreeBSD

Complete `make test' support
ClosedPublic

Authored by AMDmi3 on Sep 16 2015, 1:01 AM.

Details

Summary

Many ports have bundled tests which are mostly not used by the framework. This is obviously disappointing as it takes a great chance to discover problems early from us. Tests could also make adoption of new architectures easier, as though arm and mips support is developing, not many people have actualy software and/or care to set up qemu-emulated jails to test their ports, while this could be done automatically.

Thus I propose to finally add full-fledged support for test target. Considerations:

  • short test name; not sure where regression-test came from, but let's keep it short and simple
  • complete target sequence similar to build/install: test-message, test-depends, pre-test, do-test, post-test. pre- do- post- targets overridable from individual makefiles as usual, default targets provided where possible
    • perl framework already provides test target, I intend to be compatible with that; other frameworks/uses may do similar things
    • there's TEST_TARGET knob for ports which use usual `make'; it behaves a bit differently than ALL_TARGET/INSTALL_TARGET: it's empty by default for which no do-test target is provided, but it can be set to a target name (upsteam tend to use different names like test or check), it which case ${MAKE_CMD} ${TEST_TARGET} is called with usual make env/args
      • TEST_ENV, TEST_ARGS are provided defaulted to MAKE_ENV, MAKE_ARGS (compatible to what perl framework does); may be overridden by the port
    • individial ports may always define their own do-test: target
    • may add more standard tests later; for example, I can think of standard test for shared libs which tries to link with them. There are several known cases of broken shared libs in the tree, and stage-qa based check proved to give too many false positives
  • test depends on install. It could just depend on build, but I want to make it more flexible - for example, test may expect files arranged in a target hierarchy, e.g. stagedir. Probably, it's also convenient because it brings in run-depends which may be needed for tests as well

However, note that this code is preliminary, draft and actually a huge copypasta. Careful review is needed. Unsolved issues:

  • dependency handling; the Mk code for this is huge and I don't understand it completely
    • what lists should TEST_DEPENDS be included into?
    • what auxilitary targets are needed (pretty-print-test-depends? others)?
    • should BUILD/RUN/LIB depends be added to TEST_DEPENDS at some point
    • ...
  • probably needs more documentation
  • there are ports which need extra BUILD_DEPENDS for tests (see devel/pire). I'd like to handle these consistently, for which a dedicated OPTION should be defined (e.g. TESTS) and used in all ports with these conditions. This may be done separetely from this review though.
  • poudriere interaction should be kept in mind. Obviously poudriere testport and poudriere bulk -t should run tests.
Test Plan
  • I've tested this on my own devel/sdl2pp port with TEST_TARGET=test and some artifical TEST_DEPENDS. This works fine - make test build and stages the port, then installs dependency, then runs test which succeeds
  • Tested perl compatibility on p5-Module-Build (no changes to the port needed). Likewise, this works fine.
  • Tested on a random port without tests, this also works fine not trying to run any tests as expected

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

AMDmi3 updated this revision to Diff 8785.Sep 16 2015, 1:01 AM
AMDmi3 retitled this revision from to Complete `make test' support.
AMDmi3 updated this object.
AMDmi3 edited the test plan for this revision. (Show Details)
AMDmi3 added reviewers: bapt, bdrewery.
AMDmi3 updated this revision to Diff 8786.Sep 16 2015, 1:09 AM
AMDmi3 edited edge metadata.
  • Remove debug stuff
  • Add test support to devel/sdl2pp
AMDmi3 updated this object.Sep 16 2015, 1:10 AM
AMDmi3 edited edge metadata.
AMDmi3 updated this object.
bdrewery requested changes to this revision.Sep 16 2015, 1:12 AM
bdrewery edited edge metadata.

Thanks for working on this!

Mk/bsd.port.mk
4500–4512 ↗(On Diff #8786)

Please use Mk/Scripts/test-depends-list.sh and copy how the all-depends-list target via ALL-DEPENDS-LIST works now.

It would be nice to use less inline shell and not add more.

This revision now requires changes to proceed.Sep 16 2015, 1:12 AM
bapt edited edge metadata.Sep 16 2015, 6:38 AM

Sounds good to me, and I agree with @bdrewery about the inline stuff please replace it with a proper script.

AMDmi3 updated this revision to Diff 8805.EditedSep 16 2015, 11:01 PM
AMDmi3 edited edge metadata.
  • As suggested by @bdrewery, switch test-depends-list to external script. I've named it depends-list.sh, if I understand correctly it may be used for other *-depends-list targets. Is it correct that it doesn't recurse while all-depends-list does?
  • Switch cran.mk to new test framework as well
  • Complete documentation
  • Implement NO_TEST similar to NO_BUILD/NO_INSTALL. Useful to disable default tests for any reason.
  • Add TEST_DEPENDS to all-depends-list since all other depends are there. Is this correct?
  • Switch all my ports to new test framework for a demonstration
mat added a subscriber: mat.Sep 21 2015, 3:46 PM

I'm not sure why it needs a depends-list.sh file and why the all-depends-list.sh can't be used in its stead.

Mk/bsd.port.mk
5628 ↗(On Diff #8805)

Mmm, TEST should go before INSTALL, no ? Tests are usually not ran after installing, but are contained within WRKSRC.

5692–5693 ↗(On Diff #8805)

Could you add _OPTIONS_test and _USES_test variables ?

In D3680#76447, @mat wrote:

I'm not sure why it needs a depends-list.sh file and why the all-depends-list.sh can't be used in its stead.

This depens on whether *depends-list should recurse. I honestly don't know the answer, but since build-depends-list and run-depends-list do not, I've taken it as test-depends-list shouldn't either.

Mk/bsd.port.mk
5628 ↗(On Diff #8805)

Please read considerations in the description. There definitely will be tests which are more convenient (maybe even `only possible') to run from stagedir. For instance, I want
Mk/Uses/libtest.mk:

do-test:
    ${FIND} ${STAGEDIR}${PREFIX}/lib -name "*.so" | while read so; do \
        ${CC} -l {} -o /dev/null || ( echo "Failed to link `basename $so`"; exit 1); \
    done
5692–5693 ↗(On Diff #8805)

Sure, good catch!

AMDmi3 updated this revision to Diff 8865.Sep 21 2015, 4:25 PM
AMDmi3 edited edge metadata.

Added _OPTIONS_test and _USES_test

mat added inline comments.Sep 21 2015, 5:38 PM
Mk/bsd.port.mk
5692–5693 ↗(On Diff #8805)

Mmmm, for the options to do anything, you need to add entries to the _OPTIONS_TARGETS variable in bsd.options.mk, and for the uses, you need to add test to the .for look around line 1420 in bsd.port.mk. Sorry I was not more helpful to start with :-)

AMDmi3 updated this revision to Diff 8868.Sep 21 2015, 5:50 PM

Added test to _OPTIONS_TARGETS

Mk/bsd.port.mk
5692–5693 ↗(On Diff #8865)

Mmmm, for the options to do anything, you need to add entries to the _OPTIONS_TARGETS variable in bsd.options.mk

Done.

and for the uses, you need to add test to the .for look around line 1420 in bsd.port.mk. Sorry I was not more helpful to start with :-)

It was already there.

AMDmi3 added a reviewer: mat.Sep 22 2015, 3:51 PM
bapt accepted this revision.Sep 28 2015, 12:51 PM
bapt edited edge metadata.
antoine added a subscriber: antoine.EditedSep 28 2015, 1:34 PM

mmm this changes all-depends-list behavior, it is no longer recursive after this change...

Ah no, I mis-read the diff and didn't see all-depends-list.sh was copied to depends-list.sh

This revision was automatically updated to reflect the committed changes.