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

Update tests for Elm 0.16 #447

Merged
merged 14 commits into from Feb 25, 2016

Conversation

Projects
None yet
3 participants
@deadfoxygrandpa
Contributor

deadfoxygrandpa commented Nov 24, 2015

I don't think the core library tests need to depend on building Elm Platform from source. The build has been erroring out on travis for quite some time. I'm going to try updating everything relevant to get the tests working again.

@deadfoxygrandpa

This comment has been minimized.

Show comment
Hide comment
@deadfoxygrandpa

deadfoxygrandpa Nov 24, 2015

Contributor

Crap, I didn't check the open PRs and ended up redoing basically everything in https://github.com/elm-lang/core/pull/446, but it seems to be working now at least.

Contributor

deadfoxygrandpa commented Nov 24, 2015

Crap, I didn't check the open PRs and ended up redoing basically everything in https://github.com/elm-lang/core/pull/446, but it seems to be working now at least.

@deadfoxygrandpa

This comment has been minimized.

Show comment
Hide comment
@deadfoxygrandpa

deadfoxygrandpa Nov 24, 2015

Owner

The reasoning behind moving the node test.js line out of the script and into the travis config is because only the actual node test.js line executes the relevant tests. If anything fails before then, it's really more of a build error conceptually rather than failing tests. This change enforces that reasoning on travis.

Owner

deadfoxygrandpa commented on fb94254 Nov 24, 2015

The reasoning behind moving the node test.js line out of the script and into the travis config is because only the actual node test.js line executes the relevant tests. If anything fails before then, it's really more of a build error conceptually rather than failing tests. This change enforces that reasoning on travis.

@deadfoxygrandpa

This comment has been minimized.

Show comment
Hide comment
@deadfoxygrandpa

deadfoxygrandpa Dec 7, 2015

Contributor

I don't want to push too hard, but it would be nice to get this PR accepted. Tests have been broken for a while and this will enable them to check all the other PRs and commits again.

Contributor

deadfoxygrandpa commented Dec 7, 2015

I don't want to push too hard, but it would be nice to get this PR accepted. Tests have been broken for a while and this will enable them to check all the other PRs and commits again.

@jonathanperret

This comment has been minimized.

Show comment
Hide comment
@jonathanperret

jonathanperret Feb 25, 2016

Contributor

I hate to comment just to say 👍 but this does look like it would be very useful. I just cloned this repo hoping to contribute and was surprised to discover I couldn't run the tests.

Contributor

jonathanperret commented Feb 25, 2016

I hate to comment just to say 👍 but this does look like it would be very useful. I just cloned this repo hoping to contribute and was surprised to discover I couldn't run the tests.

evancz pushed a commit that referenced this pull request Feb 25, 2016

@evancz evancz merged commit 88cc522 into elm:master Feb 25, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Feb 25, 2016

Member

@deadfoxygrandpa, I do not have notifications on for a lot of repositories. The volume of messages is too high, so instead of always responding to whatever is going on, I periodically do a more comprehensive review of repos. If I did not do this, I feel I would make no real progress anymore.

In any case, I got a notification on this because I was cc'd maybe in a deleted comment. This is nice, thank you!

Member

evancz commented Feb 25, 2016

@deadfoxygrandpa, I do not have notifications on for a lot of repositories. The volume of messages is too high, so instead of always responding to whatever is going on, I periodically do a more comprehensive review of repos. If I did not do this, I feel I would make no real progress anymore.

In any case, I got a notification on this because I was cc'd maybe in a deleted comment. This is nice, thank you!

jonathanperret added a commit to jonathanperret/elm-core that referenced this pull request Feb 25, 2016

Ignore tests/raw-test.js generated for tests
Following the merge of #447, a file `tests/raw-test.js` is generated
during a test run. It should be ignored by Git.

jonathanperret added a commit to jonathanperret/elm-core that referenced this pull request Feb 25, 2016

Tell Travis to use a node_js environment, not Haskell
Since #447 Elm is no longer built from source to run the tests,
therefore a few seconds can probably be shaved from the Travis build by
requesting a node_js environment.

jonathanperret added a commit to jonathanperret/elm-core that referenced this pull request Feb 25, 2016

Tell Travis to use a node_js environment, not Haskell
Since #447 Elm is no longer built from source to run the tests,
therefore a few seconds can probably be shaved from the Travis build by
requesting a node_js environment.

jonathanperret added a commit to jonathanperret/elm-core that referenced this pull request Feb 26, 2016

Tell Travis to use a node_js environment, not Haskell
Since #447 Elm is no longer built from source to run the tests,
therefore a few seconds can probably be shaved from the Travis build by
requesting a node_js environment.
@jonathanperret

This comment has been minimized.

Show comment
Hide comment
@jonathanperret

jonathanperret Feb 26, 2016

Contributor

The error reported by Travis is weird:

Linking elm-stuff/packages/elm-lang/core/3.0.0 to /home/travis/build/elm-lang/core
Problem in dependency elm-lang/core 3.0.0
The elm-package.json constraints of 'elm-lang/core' are probably
letting too much stuff through. Definitely open an issue on the relevant github
repo to get this fixed and save other people from this pain.
In the meantime, take a look through the direct dependencies of the broken
package and see if any of them have had releases recently. If you find the new
thing that is causing problems, you can artificially constrain things by adding
some extra constraints to your elm-package.json as a stopgap measure.
Detected errors in 1 module.

I don't have this problem locally, nor even in Travis builds of my fork: https://travis-ci.org/jonathanperret/elm-core/builds/111891215

One difference is that Travis uses its container-based infrastructure (https://docs.travis-ci.com/user/workers/container-based-infrastructure/) for builds of my fork, because it is the default for repos recognized by Travis after 2015-01-01.

I've prepared PR #509 that forces container-based builds for this repo. The resulting build passes: https://travis-ci.org/elm-lang/core/builds/111895888. This does not explain the problem above, but at least merging #509 should allow tests to be green again on https://github.com/elm-lang/core .

Contributor

jonathanperret commented Feb 26, 2016

The error reported by Travis is weird:

Linking elm-stuff/packages/elm-lang/core/3.0.0 to /home/travis/build/elm-lang/core
Problem in dependency elm-lang/core 3.0.0
The elm-package.json constraints of 'elm-lang/core' are probably
letting too much stuff through. Definitely open an issue on the relevant github
repo to get this fixed and save other people from this pain.
In the meantime, take a look through the direct dependencies of the broken
package and see if any of them have had releases recently. If you find the new
thing that is causing problems, you can artificially constrain things by adding
some extra constraints to your elm-package.json as a stopgap measure.
Detected errors in 1 module.

I don't have this problem locally, nor even in Travis builds of my fork: https://travis-ci.org/jonathanperret/elm-core/builds/111891215

One difference is that Travis uses its container-based infrastructure (https://docs.travis-ci.com/user/workers/container-based-infrastructure/) for builds of my fork, because it is the default for repos recognized by Travis after 2015-01-01.

I've prepared PR #509 that forces container-based builds for this repo. The resulting build passes: https://travis-ci.org/elm-lang/core/builds/111895888. This does not explain the problem above, but at least merging #509 should allow tests to be green again on https://github.com/elm-lang/core .

jonathanperret added a commit to jonathanperret/elm-core that referenced this pull request Feb 26, 2016

Tell Travis to use a node_js environment, not Haskell
Since #447 Elm is no longer built from source to run the tests,
therefore a few seconds can probably be shaved from the Travis build by
requesting a node_js environment.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment