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

Separate tests #141

Merged
merged 4 commits into from
Mar 18, 2018
Merged

Separate tests #141

merged 4 commits into from
Mar 18, 2018

Conversation

ralphtheninja
Copy link
Member

@ralphtheninja ralphtheninja commented Mar 17, 2018

This cleans up the scripts a bit. I also wanted everything to be named as tests, since they are actually tests.

Running tests locally by e.g. npm test will test everything but the sauce browser tests, which can be run separately by npm run test:sauce.

Travis runs npm run test:all which tests everything.

Closes #140

Will squash.

@ralphtheninja ralphtheninja requested a review from vweevers March 17, 2018 21:47
@vweevers
Copy link
Member

Seeing the end result, I think we're moving in the wrong direction (including #134). Normally npm test is what runs all tests. A test:all script is only confusing. And ideally there's only entry point to the tests, with as much coverage as possible.

So we could also do:

"scripts": {
  "test": "standard && npm run dependency-check && cross-env DEBUG=airtap* tape test/index.js",
  "dependency-check": "dependency-check package.json --missing --unused -i dependency-check -i phantomjs -i cross-env -i standard --entry test/index.js --entry test/fixtures/*/*.js --entry frameworks/*.js"
}

And skip tests automatically, by adding this to tape-sauce.js:

if (!process.env.CI && !auth.username && !auth.key) {
  t.skip('no sauce labs credentials provided')
  return t.end()
} else if (!auth.username || !auth.key) {
  t.fail('incomplete sauce labs credentials provided')
  return t.end()
}

I'm on the fence, neither solution is 100% clean.

@ralphtheninja
Copy link
Member Author

And skip tests automatically, by adding this to tape-sauce.js:

Sounds good. I got carried away :)

@ralphtheninja
Copy link
Member Author

@vweevers Updated.

@vweevers
Copy link
Member

Weird, seems like travis built an older commit:

$ npm run test:all
npm ERR! missing script: test:all

Restarting the build.

key: auth.key,
concurrency: 5,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh that feels goood

@vweevers
Copy link
Member

@ralphtheninja mind pushing a dummy commit? Something is borked in Travis or the git history

@ralphtheninja ralphtheninja merged commit 3474396 into master Mar 18, 2018
@ralphtheninja ralphtheninja deleted the separate-tests branch March 18, 2018 19:00
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.

2 participants