rtfeldman/node-test-runner

Regression in version 0.19.1-revision4

Closed this issue · 7 comments

While updating our CI to revision 4 because of JS vulnerabilities, it is now crashing. We are generating tests files on the fly, copying them from a lot of different places all into build/tests/, then we run elm-test build/tests. It worked fine up to revision-3 but with revision-4 it gives:

Error reading elm.json: Error: /home/matthieu/git/exercism/elm/elm.json: ENOENT: no such file or directory, open '/home/matthieu/git/exercism/elm/elm.json'

This is a legitimate complain since there is indeed no elm.json where we used to run the elm-test command. There is one in the build/ directory though, which is the one picked by elm-test in versions previous to revision-4. Our issue is easily solved by running the command from the build/ directory but I thought I'd still mention that difference in behavior.

So your directory structure looks like:

./repo/
  ./build/
    ./tests/
      ./SomeElmFile.elm
    ./elm.json  

You could (previously) run elm-test build/tests from the ./repo directory and the test runner would find ./repo/build/elm.json and run correctly. Now elm-test only looks for ./repo/elm.json and fails if it cannot find it. This is probably the fault of #435 and
265017e. Quoting from that commit message:

drop dependancy on find-parent-diff

Whilst this may look like it breaks elm-test when we run from a subdirectory
of the current project, it does not. elm-test is already broken when run
from a subdirectory. This just removes code that is broken and does not work.
If anyone wants to fix this reverting this commit would not be a bad place to
start.

In the meantime, one less dependancy is a good thing. All the more so becase
https://github.com/thlorenz/find-parent-dir has not been updated for 7 years.

It seems that this feature was not entirely broken and thus I remove some functionality that you were relying on. Sorry!

Yes that's the exact setup. I don't think it was intentional though, more a consequence of spending as little time as possible on this, not realizing that I was not calling elm-test from the correct directory.

I saw from the linked PR that there is an easy work around (cd first) and this crashes rather than silently failing right?

this crashes rather than silently failing right?

Yes, if we don't run elm-test from the ./build/ dir but from ./ directly, where there is no elm.json it crashes. And indeed the work around is very easy for us. I'd even consider this more a fix than a workaround. The previous behavior is rather surprising in fact.

Yeah I am leaning towards closing this with a wont-fix label. I am very sorry that we broke this for you! I do not think I am sorry enough to add hacks back into the code base to restore the original behavior (that is pretty odd after all). Especially because the failure is pretty easy to spot and fix. How does that sound?

sounds reasonable. We do not intend to rely on that behavior anymore.

Thanks for the report @mpizenberg!