Skip to content

Conversation

jerith666
Copy link
Contributor

See #1016 for changes to the test code itself.

elm/core is special because the compiler always inserts an implicit
dependency on it. therefore it's impossible to have the tests
integrated in the elm/core project in the normal way -- if we try to
symlink out from $ELM_HOME to our project's source in the way that was
done for 0.18, the compiler gets into a deadlock. other approaches
cause it to find two copies of all the code in elm/core and refuse to
proceed.

instead, this approach creates a logically independent project, with
no real code, only tests.

these are no longer required by the new version of elm-test
* Date is no longer part of elm/core; remove tests for it

* need to import Debug.toString to test it now

* 0.19 limited tuple size, so reduce test to match

* use new modBy and remainderBy functions

* 0.19 removed flip, curry, and uncurry; so, remove tests for those

* updates for elm-test:
  * remove a duplicate test
  * use Expect.within with floats
* expose Array module contents w/o prefix rather than Maybe

* use modBy instead of (%)

* remove test for Array.toString, removed in 0.19
* shadowing is no longer allowed by the compiler; remove test for it
* elm-test now requires tests to have unique names
* update import syntax

* use String.fromInt, modBy, and Tuple.pair rather than toString, (%), and (,)

* inline our own version of flip, as it was removed in 0.19

* remove test for scanl, which was removed in 0.19
* use modBy rather than (%)

* String.toInt now returns a Maybe rather than a Result, so implement
  our own local version of the old behaviour, so the andThen tests can
  continue to build upon it.
* String.toInt and String.toFloat now return Maybe

* String.toInt no longer parses hex literals in 0.19
elm/core is special because the compiler always inserts an implicit
dependency on it.  therefore it's impossible to have the tests
integrated in the elm/core project in the normal way -- if we try to
symlink out from $ELM_HOME to our project's source in the way that was
done for 0.18, the compiler gets into a deadlock.  other approaches
cause it to find two copies of all the code in elm/core and refuse to
proceed.

instead, this approach creates a logically independent project, with
no real code, only tests.
@jerith666
Copy link
Contributor Author

jerith666 commented Feb 9, 2019

CI build failed with:

tests/run-tests.sh: line 43: node_modules/elm-test/bin/elm-test: No such file or directory
ls: cannot access /home/travis/build/elm/core/tests/.elm/0.19.0/package/elm/core/: No such file or directory
The command "bash tests/run-tests.sh" exited with 2.

Hm ... I'll probably need some more details about how the CI environment is set up to say why this isn't working.

@jerith666
Copy link
Contributor Author

Also you might want to have a look at @harrysarson's approach in jerith666#1 -- we weren't sure which was better.

@harrysarson
Copy link
Contributor

See: https://github.com/elm/core/blob/master/.travis.yml#L29

Travis is trying to run the tests using elm 0.18 which is (I think) why they are breaking.

This removes `src` as a source directory in the tests directory
as it does not exist.
Additionally, it uses a global elm-test by default in `run-tests.sh`
and uses travis elm support.

Refs: https://docs.travis-ci.com/user/languages/elm
Refs: elm#1017
@harrysarson
Copy link
Contributor

What is the status here? I note that #1016 was merged but not this one.

@turboMaCk
Copy link
Contributor

this deserves <3 and merge

@robinheghan
Copy link
Contributor

I couldn't get the run-tests.sh script to run on my machine. Changing it in the following way fixed it:

 #!/usr/bin/env bash
 
 set -o errexit;
-set -o nounset;
 
 #let the caller supply an ELM_TEST binary if desired
-if [ ! -v ELM_TEST ]; then
+if [ -z "$ELM_TEST" ]; then
     ELM_TEST=elm-test;
 fi

Not sure if this is a good fix though, my bash-foo is weak.

I'm running zsh on the latest Mac OS.

@jerith666
Copy link
Contributor Author

I developed this script on NixOS, which is at bash 4.4 right now. A comment on this stackoverflow answer says that -v support was added in bash 4.2. Other googling indicates that OS X is rather behind in terms of the bash that it ships with (maybe even still 3.x?).

So yeah, making it run on older bashes seems fine. :-)

If you do if [ -z "${ELM_TEST:-}" ]; then instead, you can keep the set -o nounset safety net. The :- suffix on the variable name means to use a default value if the variable is unset. (The default value is whatever follows the :-, so the empty string in this case.)

@robinheghan
Copy link
Contributor

Now it works, thanks!

Would you mind adding a line at the bottom that removes the *.dat files?

Also, is it possible to detect if the symlink already exists? It would be awesome to skip the "seeding" phase as it would greatly increases the feedback loop.

Another suggestion is to pass on the arguments to elm-test. That way one could do ./run-tests.sh --watch if one is developing things.

@evancz
Copy link
Member

evancz commented May 17, 2019

Thank you @Skinney for reviewing, and thank you @jerith666 for the fixes and patience! The suggestions made by @Skinney are probably best made separately.

@evancz evancz merged commit e2b18de into elm:master May 17, 2019
jerith666 added a commit to jerith666/core that referenced this pull request May 23, 2019
@jerith666
Copy link
Contributor Author

Also, is it possible to detect if the symlink already exists? It would be awesome to skip the "seeding" phase as it would greatly increases the feedback loop.

I don't feel like I'm familiar enough with the way elm and elm-test interact with elm-stuff/ and .elm/ to say for sure when it's safe to skip that step. If you've made changes to the tests? Changes to the elm/core code? Changes to the dependencies or test dependencies? Especially with your addition of --fuzz=1, which really sped it up for me, it seems like it's safest to just always run the seeding phase.

@robinheghan
Copy link
Contributor

Yeah, you're right. I was thinking that perhaps the link could be removed at the end of the script, and then at the beginning of the script we check if the link is actually there before seeding. But after fuzz=1 i don't mind to terribly.

harrysarson added a commit to harrysarson/json that referenced this pull request Jan 4, 2020
With this commit, tests run again (I believe they last run with
elm 0.18.0). I have used the same approach as
elm/core#1017 to get these tests running.
@harrysarson harrysarson mentioned this pull request Jan 4, 2020
marc136 pushed a commit to elm-janitor/json that referenced this pull request Nov 25, 2023
fix tests

With this commit, tests run again (I believe they last run with
elm 0.18.0). I have used the same approach as
elm/core#1017 to get these tests running.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants