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

Not reloading file when repeatedly finding tests #57

Open
NoahTheDuke opened this issue Aug 14, 2018 · 6 comments
Open

Not reloading file when repeatedly finding tests #57

NoahTheDuke opened this issue Aug 14, 2018 · 6 comments

Comments

@NoahTheDuke
Copy link

Currently, require-namespaces-in-dir doesn't reload the vars in a given namespace or directory of namespaces. This is an issue when running tests from a REPL and find-tests doesn't see new or modified tests.

This line is the issue. If (require ns) were to be instead, (require (into [] (concat ns [:reload :all]))), it would (probably) fix the issue and find new and updated tests.

Unless I've missed something, in which case, anyone care to tell me how to do this?

@NoahTheDuke NoahTheDuke changed the title Not reloading file when rerunning tests Not reloading file when repeatedly finding tests Aug 14, 2018
@weavejester
Copy link
Owner

require-namespaces-in-dir requires namespaces in a directory, as the name rather implies! 😃

Reloading files is outside the scope of the function, and it's better to use a library like tools.namespace or some other namespace tracker/reloader to handle it.

@NoahTheDuke
Copy link
Author

I guess I wonder how useful it is to use in the REPL if it can’t discover new or modified tests in existing files. Without auto-running (test-refresh style), the REPL is the best means of quickly checking the tests.

However, if you feel this is outside the scope of the library, I guess that’s that and I’ll just create a wrapper for this functionality.

@weavejester
Copy link
Owner

If we were to add functionality like this in, then it would need to be a new function, and essentially it would be a wrapper of the refresh function in tools.namespace. I'm uncertain whether that's a good idea or not right now.

Using :reload-all to refresh namespaces is prone to error when used repeatedly, because it doesn't remove old vars. This is especially important when testing, since if your code relies on a var that has been deleted, your tests won't fail if that var is still hanging around in memory.

@Deraen
Copy link
Contributor

Deraen commented Aug 21, 2018

Bat-test contains implementation of tools.namespace eftest integration: https://github.com/metosin/bat-test/blob/master/src/metosin/bat_test/impl.clj#L127

The code uses low level c.t.n functions (dir and track namespaces) of so that it can also use the changed file information to only run tests in the changed namespaces. Important parts are

  1. Init tracker, track/tracker
  2. dir/scan-dirs to find files and changes
  3. custom logic to select which namespaces to load
  4. reload namespaces, reload/track-reload
  5. run tests in reloaded namespaces

Currently bat-test doesn't contain REPL API but I've been thinking about that: metosin/bat-test#21

@weavejester
Copy link
Owner

Your implementation is more complex than the one I was thinking of:

(def test-path "test")

(defn test-dir []
  (run-tests (find-tests test-path))

(defn refresh-and-test []
  (require-namespaces-in-dir test-path)
  (repl/refresh :after `test-path))

With some additional functions for setting a REPL test path.

@Deraen
Copy link
Contributor

Deraen commented Aug 21, 2018

Yes, bat-test code is more complex as it uses lower level c.t.n. functions so it can only run tests in the changed namespaces. reload-and-test logic is similar to repl/refresh.

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

No branches or pull requests

3 participants