rtfeldman / node-test-runner

Runs elm-test suites from Node.js. Get it with npm install -g elm-test
BSD 3-Clause "New" or "Revised" License
132 stars 79 forks source link

Fix install-unstable-test-master for packages #612

Closed lydell closed 2 years ago

lydell commented 2 years ago

@jfmengels reported this error when trying elm-test install-unstable-test-master on a package:

npx elm-test install-unstable-test-master
Could not find "elm-explorations/test": "1.2.2" in your elm.json file here:

/home/jeroen/dev/elm-review-unused/elm.json

This command only works if you have elm-explorations/test as a (direct) test-dependency,
and only if you use version 1.2.2.

You seem to be using version 1.2.2 <= v < 2.0.0.

For some reason, Elm uses version ranges event for test-dependencies for packages. I assumed test-dependencies were always single versions, and never tested packages.

This PR adds supports for packages, in a cheat way. Rather than implementing range calculations, we simply require the exact string "1.2.2 <= v < 2.0.0": (edit: we also allow whitespace differences)

harrysarson commented 2 years ago

Looks good, one question: what if users have the string "1.2.2 <= v < 2.0.0" (note the extra space)? Could the error message be confusing for them?

lydell commented 2 years ago

Elm is super strict about the spaces:

❯ elm make
Dependencies ready!
-- PROBLEM WITH CONSTRAINT -------------------------------------------- elm.json

I got stuck while reading your elm.json file. I do not understand this version
constraint:

13|         "elm/project-metadata-utils": "1.0.0  <= v < 2.0.0"
                                                 ^
I need something like "1.0.0 <= v < 2.0.0" that explicitly lists the lower and
upper bounds.

Note: The spaces in there are required! Taking them out will confuse me. Adding
extra spaces confuses me too. I recommend starting with a valid example and just
changing the version numbers.

elm-json doesn’t like the extra space either:

-- INVALID ELM.JSON ------------------------------------------------------------

Invalid range: 1.2.0  <= v < 2.0.0

However, adding an extra space in "test-dependencies" seems to be fine – I think it’s because our dependency solver is not strict about spaces. (The generated elm.json gets something like "elm-explorations/test": "1.2.2" in it.)

So at first I was going to answer that it won’t happen, but now I’m not quite sure. I guess we could remove all whitespace from both actual and expected before comparing if we want.