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

Add a verification step to fail the build when tests are filtered with .only #4035

Merged
merged 1 commit into from May 27, 2015

Conversation

marcioj
Copy link
Contributor

@marcioj marcioj commented May 8, 2015

Previously I sent a PR with a missing .only in the tests, this made the whole build just run tests of one file. To prevent this problem I added a verification step as requested by @stefanpenner.

@marcioj
Copy link
Contributor Author

marcioj commented May 8, 2015

Tests passed in my machine not sure what is happening here. Maybe this error is something related to travis.
Can someone please purge the cache and restart the build?
Thanks

In the end it wasn't related to travis cache.

@marcioj marcioj force-pushed the avoid-only branch 4 times, most recently from 151b92a to 04e36de Compare May 9, 2015 20:56
@marcioj
Copy link
Contributor Author

marcioj commented May 9, 2015

When I changed addFiles(mocha, '/**/*-test.js'); addFiles(mocha, '/**/*-slow.js'); to a single call like addFiles(mocha, '/**/*{-test.js,-slow.js}');. I noticed that some tests failed, so I reverted the changes.
Is there some known issue with tests ordering?
Anyway the travis tests are passing now.

@marcioj
Copy link
Contributor Author

marcioj commented May 26, 2015

Friendly ping

@stefanpenner
Copy link
Contributor

Friendly ping

sorry, i must have missed this one.

@stefanpenner
Copy link
Contributor

Is there some known issue with tests ordering?

not that i know of :S, maybe some issue do exist?

Is this ready to go?

@marcioj
Copy link
Contributor Author

marcioj commented May 27, 2015

sorry, i must have missed this one.

No problem :)

not that i know of :S, maybe some issue do exist?
Is this ready to go?

I remember when I changed the glob expression to /**/*{-test.js,-slow.js} some tests failed, the first thing that comes to my mind was the test ordering, because it was the only difference I noticed.

Anyway the changes related to .only in tests are ready to go.

@marcioj
Copy link
Contributor Author

marcioj commented May 27, 2015

Wait a second, I'm unsure why I added package.json and bower.json in the addon blueprint. Let me see what happens if I remove them.

@marcioj
Copy link
Contributor Author

marcioj commented May 27, 2015

@stefanpenner now it's ready to go

stefanpenner added a commit that referenced this pull request May 27, 2015
Add a verification step to fail the build when tests are filtered with .only
@stefanpenner stefanpenner merged commit 19d49bf into ember-cli:master May 27, 2015
@stefanpenner
Copy link
Contributor

thank you kindly sir :)

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.

None yet

2 participants