-
Notifications
You must be signed in to change notification settings - Fork 360
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
fix tests by making tests a package instead of an application #1104
base: master
Are you sure you want to change the base?
Conversation
f3bef63
to
622de43
Compare
Should be ready for review now :) |
Marking as draft because we have found a better fix in #1103 |
f450b6e
to
5ced156
Compare
CI passes now: https://travis-ci.org/github/harrysarson/core |
tests/run-tests.sh
Outdated
VERSION_DIR="$(ls ${ELM_HOME}/0.19.1/packages/elm/core/)" | ||
CORE_PACKAGE_DIR="${ELM_HOME}/0.19.1/packages/elm/core/$VERSION_DIR" | ||
# Create a link to the git package | ||
CORE_LINK="${ELM_HOME}/0.19.1/packages/elm/core/1.0.5" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might want to add a comment here saying that setting this to 1.0.5
is fine as long as the strategy used by the dependency solver in elm-test (here elm-json solve
) prioritizes installed versions. In such case, it will use the linked 1.0.5
to the git repo even when a new 1.0.6 version of core exists. This of course only holds true as long as the build-time and run-time tests dependencies do not require a version of elm/core
> 1.0.5
.
Otherwise, we need to bump this 1.0.5
once there is a new release of elm/core
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about this: a888bfe hopefully this will keep us in sync regardless of the dependency resolution
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose this does the trick in 99% of the cases. The only exception I can think of is for the commit that changes the package version. It will try to use the new version that does not exist in the package server yet and thus will make the elm compiler fail I think. Might be annoying if this shows a red failure for CI on the commit corresponding to new package push.
What do you think?
PS: except if your elm publish
is faster than your CI but that's a bit playing the daredevil ^^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, this is a problem. Maybe we need to go back to the old approach of running elm-test once to prepopulate elm_home and then simlink
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well you could also just ask the package server which version of elm-core is the latest.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
edited
curl -L https://package.elm-lang.org/packages/elm/core/releases.json | jq -r 'keys | .[-1]'
# 1.0.5
I would like to add this PR to https://github.com/elm-janitor/core. I think this would be better squashed to a single commit with |
I have given you commit rights on my fork, please feel free to squash these commits on the harrysarson:fix-tests branch. |
Fix tests by making the "fake application" in ./tests instead a "fake package" which helps work around the fact that elm-json does not like the fact that does not like the hacks we use to test this repo.
Old commit message
the node test runner does not like the hacks we do in./tests/run-tests.sh. However elm-test-rs does not mind, it seems easier
to switch test runner than to update the hacks for the node test runner.
As an added extra we migrate CI from travis to github actions.