lambdaisland / kaocha

Full featured next gen Clojure test runner
https://cljdoc.org/d/lambdaisland/kaocha/1.0.861/doc/1-introduction
Eclipse Public License 1.0
792 stars 81 forks source link

`:as-alias` in custom reporter namespace prevents target namespace tests from loading #390

Closed colin-p-hill closed 1 year ago

colin-p-hill commented 1 year ago

In certain circumstances, a custom reporter which should be valid can cause a test to fail to load. A minimal repro can be found here.

It's a bit of a corner case. Here are the unusual-but-valid choices that led to this scenario:

  1. Tests are defined inline in src.
  2. The project hosts its own custom reporter.
  3. The reporter namespace has an as-alias require of one of the project's other namespaces, the latter containing tests.

This results in Kaocha not running the tests in the aliased namespace. However, they appear in the test plan when printed. When running in --watch mode, these tests do not run initially, but they do run when the watcher refreshes the namespace that contains them.

To justify one of those choices: My use case for the :as-alias is to alter reporter behavior based on a namespaced metadata flag. For portability, I wanted to use a namespace which does not make specific reference to Kaocha. This meant it had to be a different namespace than the one that holds my custom reporter, since the reporter is of course coupled tightly to Kaocha – hence, :as-alias.

I haven't tested without criterion 1, but I suspect that it isn't strictly a necessary condition. If I had separate src and test directories, I would guess that an :as-alias pointing to a test namespace would result in skipping that namespace's tests – but I think this is probably a less reasonable thing to do, so I'm presenting a more realistic use case.

A workaround exists in simply using the full namespace in my references to the keyword, rather than an autoresolving alias. But this was very surprising behavior, and the root cause of the error is very much not obvious – it took about two hours to debug, which involved starting with my entire codebase and deleting parts until I was left with the minimal case. Hate for someone else to run into the same mystery.

alysbrooks commented 1 year ago

Thank you for the issue reproduction! I was able to observe the same behavior on my machine. I tried it with a few prior versions of Kaocha because I recently refactored some of our code around configuration loading, which could plausibly affect namespace loading. The issue persisted.

However! I did notice the issue doesn't appear on Clojure 1.10.1. I then realized :as-alias wasn't available in Clojure 1.10.1, so presumably some of our loading code was never tested with :as-alias. There might be more bugs around it than the one you found.

I also tried Clojure 1.12.0-alpha1 because that would suggest it's a bug in Clojure itself that's been fixed.

alysbrooks commented 1 year ago

I'm not sure the reporter part is necessary. It seems like you get the same behavior if you move the test into another namespace required from repro.core using :as-alias. I wonder if Kaocha gets a false positive that a namespace is loaded already when the namespace has first been required using :as-alias.

Looking into it further, it appears we're using find-ns to determine whether to load a namespace, but this doesn't work when the alias has been created. I created a quick fix you can test like so:

{lambdaisland/kaocha {:git/sha "42ed542ebcd60ece0b093b4d729a3d790179d505"
                                          :git/url "https://github.com/lambdaisland/kaocha"}}
colin-p-hill commented 1 year ago

Verified in my repro and in my real project that this fixes it. Thanks for the quick patch!

alysbrooks commented 1 year ago

The fix is merged and released!

[lambdaisland/kaocha "1.77.1236"]
{lambdaisland/kaocha {:mvn/version "1.77.1236"}}