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

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

Closed
colin-p-hill opened this issue Jan 30, 2023 · 4 comments · Fixed by #392
Closed
Assignees

Comments

@colin-p-hill
Copy link

colin-p-hill commented Jan 30, 2023

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
Copy link
Member

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
Copy link
Member

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"}}

@alysbrooks alysbrooks self-assigned this Jan 31, 2023
@colin-p-hill
Copy link
Author

colin-p-hill commented Jan 31, 2023 via email

@alysbrooks
Copy link
Member

The fix is merged and released!

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants