cognitect-labs / test-runner

A test runner for clojure.test
Eclipse Public License 2.0
288 stars 32 forks source link

Fix for #17 loads too many namespaces when using -n option #29

Closed borkdude closed 3 years ago

borkdude commented 3 years ago

This commit https://github.com/cognitect-labs/test-runner/commit/c1a74fc51d40404f1b18a5b0e1631d281a7e59a4 broke my test setup.

I'm using the following script:

#!/usr/bin/env bash

if [ "$POD_DB_TYPE" = "postgresql" ]
then
    clojure -M:test -n pod.babashka.postgresql-test
fi

if [ "$POD_DB_TYPE" = "hsqldb" ]
then
    clojure -M:test -n pod.babashka.hsqldb-test
fi

if [ "$POD_DB_TYPE" = "mysql" ]
then
    clojure -M:test -n pod.babashka.mysql-test
fi

if [ "$POD_DB_TYPE" = "oracle" ]
then
    clojure -M:test -n pod.babashka.oracle-test
fi

if [ "$POD_DB_TYPE" = "mssql" ]
then
    clojure -M:test -n pod.babashka.mssql-test
fi

I am relying on the -n foo setting to not load any other namespaces, but it currently does, which causes my tests to fail whereas before the commit it worked as intended.

puredanger commented 3 years ago

As per your request in #17, I made the change there to make the test-runner help doc accurate - the help says:

-r, --namespace-regex REGEX Regex for namespaces to test. Defaults to #".*-test$"

and

All options may be repeated multiple times for a logical OR effect.

I thought about turning off the default for -r if -n is used but that seems contrary to the default stated above.

An alternative for you would be to specify a -r that doesn't match anything (overrides the default).

borkdude commented 3 years ago

Despite what the previous docs say, can't we agree that it makes most sense to only execute namespace foo when you provide the option -n foo?

The reason why I posted the issue was for cases like -n foo -r 'bar\*': execute namespace foo and bar_test.

borkdude commented 3 years ago

To re-iterate. Here is how I see it.

1) Invoking the test runner without any args should do something sensible out of the box. This is a reason for defaulting the regex option to .*_test. The default doesn't make sense on its own, it's just there to implement sensible behavior when invoked with no args. 2) When a user provides options, the default behavior of the test runner should be overridden completely, as it doesn't make sense to run all the .*_test namespaces if you make a specific selection.

puredanger commented 3 years ago

Ok, I modified to use the default regex only if neither -n or -r is supplied

borkdude commented 3 years ago

@puredanger Thanks a lot!