Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Excluding and/or including certain tests via metadata #126

Open
pithyless opened this issue Sep 4, 2021 · 11 comments
Open

Excluding and/or including certain tests via metadata #126

pithyless opened this issue Sep 4, 2021 · 11 comments
Assignees
Labels
cli Related to Polylith cli core Related to core functionality of Polylith improvement Not a bug but an improvement to overall user experience

Comments

@pithyless
Copy link

Is your feature request related to a problem? Please describe.

I would like to write tests that should not always be run, eg:

  • benchmarking/performance tests that take too long to be run every time
  • integration tests with external systems that rely on working VPN, etc. and cannot be run in all environments.

Describe the solution you'd like

The cognitect.test-runner supports this via keyword options:

  :includes - coll of test metadata keywords to include
  :excludes - coll of test metadata keywords to exclude

The filter is applied to which vars will be run: https://github.com/cognitect-labs/test-runner/blob/334f2e2a5491747dff52f9ca8cbb4a53f54d344d/src/cognitect/test_runner.clj#L27-L32

I think a solution could be to similarly support filtering out specific metadata keywords before generating the final test-statements (after all the other logic for which bricks have changed, etc):

test-statements (mapv ->test-statement (concat brick-test-namespaces project-test-namespaces))

The other question would be where to configure the inclusion/exclusions. Perhaps a combo of having defaults configured in the workspace.edn and support for passing specific exclusion/inclusion keywords via poly test?

Describe alternatives you've considered

Right now, these kind of tests:

  • need to be commented out and run manually (e.g. via a REPL inside a rich-comment-form)
  • defined in a separate project that will need to be excluded by default in poly tooling
  • perhaps not defined as tests but separate main executables that are run from root (but then we're reinventing the test-runner framework for reporting, etc)

I would prefer if they would be first-class deftest vars that are tagged via metadata and a way of excluding/including the tags via the poly test CLI. Bonus points if we can have poly CLI report that X number of tests were skipped.

Additional context

Kaocha test-runner also has a similar feature via:

Kaocha supports both filtering via metadata on the test-var and on the namespace. I would not consider the latter strictly necessary, but it is interesting to consider.

@imrekoszo
Copy link
Contributor

While it is probably useful to make Polylith's built-in test runner somewhat flexible, if I was the maintainer of Poly, I wouldn't put too much focus on that. There are several test runners available to Clojure already and we can only expect more to be developed. Those projects are focused on making the test runner experience the best possible - integration with test types, reporting, coverage etc. There's a great blog post from Lambdaisland discussing this topic.

I think Polylith's main focus should be the management of all the various projects in the workspace, and it should try and integrate with and delegate tasks to existing tooling outside of its main area of responsibility.

Test runners are a good case in my opinion. Both Kaocha and Cognitect's own have good configurability so if poly can produce the proper parameters, those can then do the job of discovering and running tests within the bounds specified by poly.

One more reason for this is that teams might already have their preferred test runner solution (with static config/plugins to integrate with their ci etc. processes), be it one of the above mentioned or some other tool (internal tool even). If poly is able to integrate with those then it's less friction for those teams to adopt Polylith.

@seancorfield
Copy link
Contributor

seancorfield commented Sep 10, 2021

@imrekoszo The main problem is that every test runner out there is different so integrating poly with any of them would be a challenge, especially since poly needs to manage classpath isolation so that tests can be run in different classpath contexts for each project. In addition, not all of those test runners provide a suitable API to be invoked with just specific tests and/or just specific directories.

@pithyless I think perhaps profiles would be something to look at for putting benchmarking, performance, and integration tests into a separate component and bringing them into the context when you want to run them. I agree it's not as flexible as using metadata and keywords -- and that would be a nice enhancement for poly test. [Updated: I thought maybe you could add arbitrary :extra-paths with a profile but, having played around with that via profiles, it seems to require a full brick structure... still, having a separate component for such only-run-these-sometimes tests does seem to be a workable approach!]

@tengstrand tengstrand added the improvement Not a bug but an improvement to overall user experience label Sep 11, 2021
@tengstrand tengstrand self-assigned this Sep 11, 2021
@imrekoszo
Copy link
Contributor

@seancorfield I didn't mean the poly team should do all of this, I think it would be enough to make it pluggable some way. One possibility would be a command that returns all the context info (classpath etc) about projects whose tests need to be executed so teams can use that to call their test runners.

@furkan3ayraktar
Copy link
Collaborator

Currently, Polylith finds all the test namespaces that were affected by the latest changes and uses cognitect-labs/test-runner to run those tests. The tests are run in the correct context, e.g. in the context of a specific project. The dependencies are resolved via tools.deps and tests run in isolation within a new classloader.

It could be unnecessary for developers to repeat all those in order to use another test runner. An idea could be that Polylith prepares everything to run the tests, and the user of Polylith could provide a Clojure function that is used to run the tests. This way, any other test runner library could be utilized.

Also, we could make the default test runner utilize this functionality and hence make the Cognitect's test runner dependency more transparent. At the moment, it is hidden in the codebase and injected on runtime.

@furkan3ayraktar furkan3ayraktar added core Related to core functionality of Polylith cli Related to Polylith cli labels Sep 11, 2021
@imrekoszo
Copy link
Contributor

@furkan3ayraktar I like that idea

@seancorfield
Copy link
Contributor

@furkan3ayraktar It doesn't use Cognitect's test-runner as far as I can see -- it just calls clojure.test/run-tests directly on the namespaces that need to be tested.

Since folks are also asking about Cloverage, I think maybe figuring out a way to provide alternate functions to call instead of clojure.test/run-tests might be a good path -- but I suspect those functions would also need the actual source path to files for some things?

Part of the problem here is that most test runners -- and Cloverage -- subsume the entire role of poly test here, so switching to a "different test runner" means giving up all of the affordances of Polylith's "test runner" in terms of per-project classpaths, isolated classloaders, and incremental testing. The underlying core is simply clojure.test and most test runners add value by hooking into that and modifying the reporting machinery (although that machinery is poorly-designed for extension, to be honest).

Some useful things that poly test could do that other "test runners" seem to provide are a) the ability to randomize the order of tests being run (at least randomize the namespaces) and b) the ability to run tests in parallel (which I question as a general principle since so many tests are stateful but...).

@yenda
Copy link

yenda commented Nov 17, 2021

@seancorfield one very useful thing that poly test could do as well is being able to generate a junit report. It's part of clojure.test and as simple as:

(binding [test/*test-out* (io/writer (io/file output "junit.xml"))]
            (junit/with-junit-output (apply test/run-tests nses)))

but currently the way poly test works one cannot generate a test report.

@imrekoszo
Copy link
Contributor

Polylith finds all the test namespaces that were affected by the latest changes

@furkan3ayraktar I think there would be value in finding all affected namespaces, not only test ones and passing them over to the test runner. There could be tests hidden in any of those (like kaocha's auto-generated s/fdef checks) and the test runner should be responsible for discovering them.

The built-in test runner could further filter that namespace list to test namespaces only and run clojure.test-s in them.

@furkan3ayraktar
Copy link
Collaborator

Related to #196

@seancorfield
Copy link
Contributor

Given that we now have pluggable test runners for Polylith and we have both an in-process Kaocha runner and an external (out-of-process, isolated) "regular" test runner -- in addition to the default, built-in one -- this issue has, I think, gotten more complex to solve in a generic way but much easier to solve in a specific way: in other words, I think this issue should be closed and if the OP wants this feature for a specific runner, open a new issue against that?

I'd be open to providing some way to filter tests in my external test runner project (but it would need to be via env vars, JVM properties, or a config file -- to avoid any complexity of trying to pass arguments through poly test into the various third-party runners). There's precedent for this already because both the Kaocha runner and my external runner support configuration that does not involve Polylith itself.

@tengstrand
Copy link
Collaborator

This is now supported by the External test runner test runner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Related to Polylith cli core Related to core functionality of Polylith improvement Not a bug but an improvement to overall user experience
Projects
None yet
Development

No branches or pull requests

6 participants