Conversation
This reverts commit b7fb43f.
gniezen
left a comment
There was a problem hiding this comment.
This is awesome! I've added a couple of suggestions and comments, but nothing major. Code quality is 💯
.circleci/config.yml
Outdated
There was a problem hiding this comment.
| - run: yarn run spectron-test | |
| - run: yarn spectron-test |
appveyor.yml
Outdated
There was a problem hiding this comment.
| - yarn run spectron-test | |
| - yarn spectron-test |
package.json
Outdated
There was a problem hiding this comment.
We've migrated from mocha to jest a while ago. I'm just wondering whether the spectron tests will work with jest, or is it only possible with mocha?
There was a problem hiding this comment.
whoops, didn't realize that, totally will work with jest, will make it so!
test/login.spec.js
Outdated
There was a problem hiding this comment.
Is this actually async? I don't see an await.
test/login.spec.js
Outdated
There was a problem hiding this comment.
I remember we had an issue for a while where the support link wasn't working. Can we also add tests here to check that the URLs don't 404?
|
|
||
| global.before(() => { | ||
| should(); | ||
| use(chaiAsPromised); |
There was a problem hiding this comment.
Would you mind adding a comment here to explain why chaiAsPromised is necessary here?
There was a problem hiding this comment.
this won't be here if we use jest i think.
|
Went ahead and switched to jest...a couple things here..
special thanks to @krystophv and @gniezen for help on this |
gniezen
left a comment
There was a problem hiding this comment.
Looks good, great job @ginnyyadav ! I just have two comments:
runis not necessary when usingyarn- Please remove the
app/package-lock.jsonfile, as that as created bynpm.
Fix spelling of spectron Co-authored-by: Gerrit Niezen <gerrit@tidepool.org>
…uploader into integrate-spectron
Simple spectron integration with a very simple test example so we can build off of it.