Fix mismatched app and test versions for indirect dependencies #653
Conversation
elm-test was running tests with indirect dependencies always at latest available version, regardless of whether the app under test had indirect dependencies pinned to an earlier version. This can lead to tests behaving different from real world apps. The reason is elm-solve-deps always picks the first acceptable version given to it in the versions array. This commit fixes the issue by ensuring indirect dependencies' versions are always sorted in this order: - the pinned version first - versions higher than the pinned version, in ascending order - versions lower than the pinned version, in descending order
|
I think CI is stuck. I noticed this locally too, but thought it was some weirdness on my computer. Mocha never exits, just hangs after reporting success. |
|
Turns out it’s due to these lines: node-test-runner/lib/DependencyProvider.js Lines 11 to 13 in 1f19768 I’m currently looking at moving those lines to a function that will be called by the real code but not by tests. |
|
Ah, requiring |
|
oops hadn't seen your message 😅 |
|
Pushed a commit that lazily initializes the worker. Now the tests finish. What do you think about the approach? |
| // Poor man’s type alias. We can’t use /*:: type SyncGetWorker = ... */ because of: | ||
| // https://github.com/prettier/prettier/issues/2597 | ||
| const SyncGetWorker /*: { | ||
| get: (string) => string, | ||
| shutDown: () => void, | ||
| } */ { | ||
| } */ = { | ||
| get: (string) => string, | ||
| shutDown: () => {}, | ||
| }; |
There was a problem hiding this comment.
ha, i was trying to figure out how to do type aliases in old flow!
| function syncGetWorker() /*: typeof SyncGet.SyncGetWorker */ { | ||
| if (syncGetWorker_ === undefined) { | ||
| syncGetWorker_ = SyncGet.startWorker(); | ||
| } | ||
| return syncGetWorker_; | ||
| } |
There was a problem hiding this comment.
Nice, clean workaround.
|
The macOS jobs get stuck without changes as well: #654 So I’m gonna merge this anyway, and make a release tomorrow. |
|
Thank you! Really appreciate the help with the stuck jobs 💜 |
2 participants
Fixes #652
This implements behavior described in the "A fix" section of the issue linked above: