refactor(js-client): convert fxa-js-client and its tests to commonjs #2039
Conversation
|
🎆 🎆 🎆 🎆 🎆 🎆 🎆 🎆 🎆 🎆 🎆 🎆 🎆 🎆 🎆 🎆 🎆 🎆 🎆 🎆 🎆 🎆 |
| @@ -1,9 +0,0 @@ | |||
| /* This Source Code Form is subject to the terms of the Mozilla Public | |||
| @@ -51,11 +52,12 @@ | |||
| "grunt-open": "0.2.4", | |||
| "grunt-webpack": "3.0.2", | |||
| "http-proxy": "1.11.1", | |||
| "intern-geezer": "2.2.3", | |||
packages/fxa-js-client/package.json
Outdated
| "test": "grunt test", | ||
| "test-local": "intern-client config=tests/intern auth_server=LOCAL", | ||
| "test": "mocha tests/lib --reporter dot", | ||
| "test-local": "AUTH_SERVER_URL=http://127.0.0.1:9000 mocha tests/lib --reporter dot", |
There was a problem hiding this comment.
I get some failures running this command.
|
@vladikoff Do you have train you are targeting this for and an estimate? |
vladikoff
force-pushed
the
mocha-js-client
branch
from
1494e82 to
bc0a2a9
Compare
vladikoff
force-pushed
the
mocha-js-client
branch
from
bc0a2a9 to
51bb7da
Compare
vladikoff
force-pushed
the
mocha-js-client
branch
from
fa86212 to
55ea72e
Compare
| ErrorMocks.signInBlocked | ||
| ); | ||
| }) | ||
| .then(assert.notOk, function(error) { |
There was a problem hiding this comment.
This is curious. assert.notOk is not supposed to be executed is it, so did it need to change? Are the assertions below still being executed?
There was a problem hiding this comment.
From the documentation of isNotOk (which according to the source is just an alias for notOk, assert.notOk will only fail of the value passed to it is falsy. That means @philbooth is right, the error handler is not guaranteed to be called here. Any time assert.notOk is used as the success when we do not expect it to be called should be an assert.fail.
vladikoff
force-pushed
the
mocha-js-client
branch
from
55ea72e to
f7d6775
Compare
There was a problem hiding this comment.
I'm very excited for this! The comment @philbooth made about assert.notOk being used for the success handler is spot on. If we do not expect the success handler to be called, those need to be assert.fail or else the error handler is not guaranteed to run.
| service: 'sync', | ||
| redirectTo: 'https://sync.127.0.0.1/after_reset', | ||
| resume: 'resumejwt', | ||
| style: 'trailhead', |
There was a problem hiding this comment.
All emails use "trailhead" style now, right?
| assert.ok(resp); | ||
| }, | ||
| error => { | ||
| console.log(error); |
| ErrorMocks.signInBlocked | ||
| ); | ||
| }) | ||
| .then(assert.notOk, function(error) { |
There was a problem hiding this comment.
From the documentation of isNotOk (which according to the source is just an alias for notOk, assert.notOk will only fail of the value passed to it is falsy. That means @philbooth is right, the error handler is not guaranteed to be called here. Any time assert.notOk is used as the success when we do not expect it to be called should be an assert.fail.
* changes client structure * moves to mocha Fixes #1857
vladikoff
force-pushed
the
mocha-js-client
branch
from
f7d6775 to
60e07b5
Compare
|
@shane-tomlinson updated! |
There was a problem hiding this comment.
R...... +
Thanks for the iterations @vladikoff, this is a great change.
Fixes #1857