Skip to content

port integration tests to jest#6518

Merged
iAmWillShepherd merged 9 commits intodevelopmentfrom
port-integration-tests-to-jest
Jan 17, 2019
Merged

port integration tests to jest#6518
iAmWillShepherd merged 9 commits intodevelopmentfrom
port-integration-tests-to-jest

Conversation

@shiftkey
Copy link
Member

@shiftkey shiftkey commented Dec 31, 2018

Overview

🌵🌵🌵builds upon #6517, go review that first 🌵🌵🌵

The (almost) last part of moving away from mocha to jest is this PR.

Description

Moving away from mocha requiring rewriting the launch test because we couldn't leverage chai-as-promised any more (which essentially promisified lots of chai's API without having to think about it). Instead this test uses spectron directly to drive the app. I upgraded to the latest spectron because we've already been talking about Electron v3 support, but this also gave me a chance to use the typings that came in the box with the spectron package.

The resulting code is more procedural but once I get some fixes upstream for the type declarations (everything is a promise-returning API it seems, despite what the typings might say) I feel much more confident in us writing more tests with it (because the magic is no longer hiding in libraries and BDD-esque syntax).

Release notes

Notes: no-notes

@shiftkey shiftkey added the infrastructure Issues and pull requests related to scripts and tooling for GitHub Desktop label Dec 31, 2018
@shiftkey shiftkey changed the title port integration tests to jest, cleanup mocha usage port integration tests to jest Dec 31, 2018
@shiftkey shiftkey force-pushed the port-integration-tests-to-jest branch 3 times, most recently from 5bf14ba to 9c8a3c7 Compare January 3, 2019 13:39
@shiftkey shiftkey force-pushed the port-integration-tests-to-jest branch from 9c8a3c7 to 00e75fe Compare January 3, 2019 17:08
@shiftkey shiftkey force-pushed the port-integration-tests-to-jest branch from 00e75fe to d62d105 Compare January 16, 2019 17:18
@shiftkey shiftkey added the ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jan 16, 2019
@shiftkey
Copy link
Member Author

I extracted the last part of the Jest migration to #6645 because I wanted to keep this PR focused on porting the tests over.

.getWindowCount()
.should.eventually.equal(1)
.browserWindow.isMinimized()
.should.eventually.be.false.browserWindow.isDevToolsOpened()
Copy link
Member Author

Choose a reason for hiding this comment

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

isDevToolsOpened() isn't exposed in the spectron typings because they just wrap electron - more context here: electron-userland/spectron#343

Copy link
Contributor

@iAmWillShepherd iAmWillShepherd left a comment

Choose a reason for hiding this comment

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

Not sure what one of the new type definitions does. Since it doesn't have an associated dependency, can we add a short comment with some words on what dependency its supporting?

@iAmWillShepherd iAmWillShepherd self-assigned this Jan 16, 2019
Copy link
Contributor

@iAmWillShepherd iAmWillShepherd left a comment

Choose a reason for hiding this comment

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

Not sure what one of the new type definitions does. Since it doesn't have an associated dependency, can we add an short comment with some words on what dependency its supporting?

@iAmWillShepherd iAmWillShepherd merged commit b7584f7 into development Jan 17, 2019
@iAmWillShepherd iAmWillShepherd deleted the port-integration-tests-to-jest branch January 17, 2019 18:13
@jtarver2018

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

infrastructure Issues and pull requests related to scripts and tooling for GitHub Desktop ready-for-review Pull Requests that are ready to be reviewed by the maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants