Skip to content
This repository has been archived by the owner on Apr 3, 2019. It is now read-only.

Test refactor #506

Merged
merged 7 commits into from Jan 21, 2014
Merged

Test refactor #506

merged 7 commits into from Jan 21, 2014

Conversation

dannycoates
Copy link
Contributor

I divided the tests into 2 types local and remote. Local tests are either unit tests of internal modules or API tests that need special configuration, for example setting a token lifetime to -1 to test expiry. Remote tests are for testing the API surface and may be run locally or against a remote server. We should try to use remote tests whenever possible.

I've moved as many API tests as possible to test/remote and the unit tests to test/local. I also started to put the tests in files by endpoint. There's some overlap, but it seems better than before.

You can test remotely like this:

PUBLIC_URL=https://api.accounts.firefox.com npm run test-remote

We now have 114 tests that run remotely 馃嵃

During development I prefer npm run test-quick which runs without code coverage.

Before pushing to github we should run npm run test-all which runs with both db backends an coverage.

r? please 馃拫

* created test/local for tests that can only be run locally
* created test/remote for tests that can run remotely or locally
* moved most api level tests to test/remote
* much more of the test suite can run remotely now :)
@@ -20,7 +20,6 @@ notifications:
skip_join: false

env:
- DB_BACKEND=memory
Copy link
Contributor Author

Choose a reason for hiding this comment

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

lets give travis a break on this one

this.server.kill('SIGINT')
this.mail.kill()
TestServer.prototype.uniqueEmail = function () {
return crypto.randomBytes(10).toString('hex') + '@restmail.net'
}
Copy link
Contributor

Choose a reason for hiding this comment

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

this is a really nice addition

@rfk
Copy link
Contributor

rfk commented Jan 20, 2014

Before pushing to github we should run npm run test-all which runs with both db backends an coverage.

In which case, should we configure travis to do the same?

@rfk
Copy link
Contributor

rfk commented Jan 20, 2014

This is really great, /cc @jbonacci to be sure he sees it. r++

@dannycoates
Copy link
Contributor Author

In which case, should we configure travis to do the same?

Maybe. I'm hesitant of allowing "dev only" code or tiny style issues to fail a PR build at this point. I guess we need to discuss what we should consider "errors" vs "warnings" and what should be automated vs manual in a review.

@jbonacci
Copy link

I saw it. :-)
馃憤

Will pass this along to the QA team.

@jbonacci
Copy link

OK. To be thorough, I verified the following:
npm test
npm run test-quick
npm run test-all

Next I ran this and got a couple of errors (will file a bug), but overall it looks great:
PUBLIC_URL=https://api-accounts.stage.mozaws.net npm run test-remote

Finally I ran this against Prod, with what looks like the very same errors.

So cool!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants