0.8.3 runs all tests on reload

[kornysietsma]

The 0.8.3 change seems to cause our tests to run whenever a reload happens.

i.e. If we run "lein ring server" the app loads happily (without running tests) If we then change a source file and refresh the browser, the server reloads and runs all our test code.

Reverting to 0.8.2 made this behaviour go away.

we have not set :reload-paths or changed our default :source-paths settings - our sources are in the "src" directory, tests are in "test" (and are Midje tests, btw)

[emil0r]

Same issue with 0.8.5

[weavejester]

Lein-Ring probably needs to be changed to ignore files in the test directories. Patches are welcome.

[davesann]

ring.server itself defaults :reload-paths to ["src"]. But lein-ring overrides this.

"lein ring server" will automatically set :reload-paths if you have not. This currently defaults to monitoring everything on your classpath. In the near term (until test paths are filtered), if you want to prevent this set :reload-paths - "lein ring server" will honour this. Setting to ["src"] should bring back the behaviour from the previous version. (untested).

[noahlz]

If one were to attempt to patch this, what would you suggest as the correct solution?

[weavejester]

Have lein-ring exclude the project test paths from the reload-paths.

[noahlz]

Do you think such a change would impact any other users, i.e. is there a valid use case for including test sources in the reload path? For example, if you run a REPL in the server?

[weavejester]

It's probably not worth having by default. People can always override it with :reload-paths.

[philc]

This behavior changed from 0.8.2 -> 0.8.3, and it's a very surprising default: my entire test suite runs after modifying the ring app. @davesann, thank you for the workaround.

@noahlz, do you plan on submitting a pull request? If so, looking forward to it!

[davesann]

I made the change that added the feature extending reloading paths beyond "src" in the parent project. I can probably fix this but there are some questions that need answers first.

The original change that I added built up the the source paths - this was changed in the final commit to use leiningen classpath - with unforeseen consequences. (a note though. I think it is probably bad practice for loading a file to runs tests directly)

The original change was here: https://github.com/davesann/lein-ring/commit/d65c2c44b1b8a784db99f73e18236328dfe68b83

I think it will be better to go back to building up the src directory list, rather than filter the classpath. I can't see how you can definitively test for a "test directory". (but we could choose to filter tests/*. This will not work in all cases).

Lein 1 and 2 projects may cause a problem because they use :source-path and :source-paths respectively.

Questions:

would you prefer to take the simple route and filter project-dir/tests/*?

if not, there are some choices in building the paths for reload:

1. a. paths only from the immediate project checkouts (i.e don't recurse)? or; b. recurse all checkouts (i.e. checkouts under "checkout projects" all the way down) ?

   I believe lein only looks to the first level.

2. a. lein2 :source-paths only? or; b. include lein2 :source-paths and lein1 :source-path as well if found?

3. should any other paths be included for reload? resources?

4. any other options?

Cheers Dave

[weavejester]

Take the simplest approach and just cut out the test paths.

Since it's customisable we don't need to think too hard about the defaults. We just need something that works in the majority of cases.

[MichaelBlume]

It's been a while -- is this still an issue?

[acrao]

This issue where test code is loaded still manifests unless you explicitly specify :reload-paths to just include your src dir. I ran into this with lein-ring 0.9.7.

[MichaelBlume]

Ok, a few things. The tests just reload, right? I would not expect reloading them to also run them unless your tests are nonstandard. @philc are your tests actually running? Anyhow, I think I would expect tests and source to both reload? Is there anything wrong with just setting :reload-paths to ["src"] if you only want that?

[weavejester]

My recollection of Midje is that merely loading the test namespace also executes the tests. I don't know if this has been fixed, or if my recollection is wrong, but if it's still the case that would explain this behaviour.

If this is right, then there's not a lot we can do about it beyond allowing the reload-paths to be changed. To my mind this is a problem with Midje, as loading namespaces shouldn't incur side effects.

[synthomat]

Haha, this weird behaviour costed me 2hrs to debug 🙃 I had tests that emptied db tables and after each (non-test) code change I was left with empty tables – super confusing.

Setting {:ring {:reload-paths ["src"]}} seems to help; maybe defaults should be changed to that (instead of reloading all files)?

[synthomat]

If this is right, then there's not a lot we can do about it beyond allowing the reload-paths to be changed. To my mind this is a problem with Midje, as loading namespaces shouldn't incur side effects.

@weavejester You mean just declaring tests in a namespace should not execute any testing code unless you actually run the test suite? I faced this problem with midje too…

[weavejester]

You mean just declaring tests in a namespace should not execute any testing code unless you actually run the test suite?

Yes, that's right. Loading a namespace should be safe.