Skip to content
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

support tests/ with elm 0.19.0 #2176

Merged
merged 4 commits into from
Jan 4, 2019
Merged

Conversation

altaurog
Copy link
Contributor

@altaurog altaurog commented Jan 2, 2019

With earlier elm versions, a separate package file is maintained for tests, which when properly configured enabled the compiler to find what it needed to compile the tests. Under elm 0.19, test dependencies are managed in the top-level package file, so elm make will fail on the tests. elm-test make is required in this case.

See elm-explorations/test#64

I am unable to test on windows currently, but it works for me in the wild, and I have added tests for the new behavior.

Also I updated the tests to test behavior when elm package file is present (which is pretty much always).

@altaurog
Copy link
Contributor Author

altaurog commented Jan 2, 2019

Okay, I see that the CI tests do run on windows, and will have a look at fixing them.
The travis CI build failure is harder to understand. I ran all the docker tests with run-test and they passed. Any suggestions on how to debug that?

@RyanSquared
Copy link
Member

Easiest way is ctrl-f and look for (X):

  Starting Vader: /testplugin/test/command_callback/test_elm_make_command_callback.vader
    (1/5) [EXECUTE] should get valid executable with default params
    (2/5) [EXECUTE] should get elm-test executable for test code with elm >= 0.19
    (2/5) [EXECUTE] (X) ['elm', 'cd ''/testplugin/test/elm-test-files/newapp'' && ''elm'' make --report=json --output=/dev/null %t'] should be equal to ['/testplugin/test/elm-test-files/newapp/node_modules/.bin/elm-test', 'cd ''/testplugin/test/elm-test-files/newapp'' && ''/testplugin/test/elm-test-files/newapp/node_modules/.bin/elm-test'' make --report=json --output=/dev/null %t']
    (3/5) [EXECUTE] should get plain elm executable for test code with elm < 0.19
    (3/5) [EXECUTE] (X) ['elm', 'cd ''/testplugin/test/elm-test-files/oldapp'' && ''elm'' make --report=json --output=/dev/null %t'] should be equal to ['/testplugin/test/elm-test-files/oldapp/node_modules/.bin/elm', 'cd ''/testplugin/test/elm-test-files/oldapp'' && ''/testplugin/test/elm-test-files/oldapp/node_modules/.bin/elm'' make --report=json --output=/dev/null %t']
    (4/5) [EXECUTE] should get valid executable with 'use_global' params
    (5/5) [EXECUTE] should get valid executable with 'use_global' and 'executable' params
  Success/Total: 3/5

@altaurog
Copy link
Contributor Author

altaurog commented Jan 3, 2019

@RyanSquared thanks. Finding the failed test (mine) wasn’t a problem. What I don’t understand is why the results of the CI would differ from the results I get when I run the docker container locally, since on the face of it, the environments should be the same. If I find more time for this, I will dig deeper. In the meantime, it works for me, at least.

@altaurog
Copy link
Contributor Author

altaurog commented Jan 3, 2019

ah. I see.

@altaurog altaurog force-pushed the feat/elm-tests branch 5 times, most recently from 4f65873 to 289e006 Compare January 3, 2019 19:46
most projects will have an elm.json file (>= 0.19) or elm-package.json (< 0.19)
With earlier elm versions, a separate package file is maintained for
tests, which when properly configured enabled the compiler to find what
it needed to compile the tests.  Under elm 0.19, test dependencies are
managed in the top-level package file, so `elm make` will fail on the
tests.  `elm-test make` is required in this case.

See elm-explorations/test#64
windows paths have backslashes, which are special in regex patterns
@altaurog
Copy link
Contributor Author

altaurog commented Jan 4, 2019

Well, this PR is ready to go. It seems that the test failures are coming from master.

@w0rp
Copy link
Member

w0rp commented Jan 4, 2019

Yeah, I'll fix those. I'm not surprised if Windows tests can fail.

@w0rp w0rp merged commit 3b96ab4 into dense-analysis:master Jan 4, 2019
@w0rp
Copy link
Member

w0rp commented Jan 4, 2019

Cheers! 🍻

@altaurog
Copy link
Contributor Author

altaurog commented Jan 4, 2019

thanks!

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.

3 participants