Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Update tests #53

Merged
merged 10 commits into from
Oct 24, 2019
Merged

Update tests #53

merged 10 commits into from
Oct 24, 2019

Conversation

kgryte
Copy link
Member

@kgryte kgryte commented Oct 24, 2019

This PR moves files around to minimize the need to rebuild when updating tests and modifies some of the test commands.

launch: {
headless: process.env.HEADLESS !== "false"
headless: process.env.HEADLESS !== 'false',
slowMo: process.env.SLOWMO === 'true'
Copy link
Member

Choose a reason for hiding this comment

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

Nice

Copy link
Member Author

Choose a reason for hiding this comment

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

Was definitely useful in seeing what button was being pressed in real-time.

},
// https://github.com/smooth-code/jest-puppeteer/tree/master/packages/jest-dev-server#options
server: {
command: "jupyter lab --port 8080 --no-browser",
command: "jupyter lab --port 8080 --no-browser --LabApp.token=''",
Copy link
Member

Choose a reason for hiding this comment

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

Ah good call

Copy link
Member Author

Choose a reason for hiding this comment

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

One less configuration file.

"outDir": "build/test",
"rootDir": "test"
},
"include": ["test/*", "test/**/*"]
Copy link
Member

Choose a reason for hiding this comment

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

Yeah probably good call to separate these

Copy link
Member Author

Choose a reason for hiding this comment

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

I recognize that, in other worlds (e.g., Go, Ruby), placing tests next to source files is common; in Node.js land, it isn't for reasons of separation of concerns. Plus, UI tests cross file divides and don't fit nicely into the foo.spec.ts paradigm where foo refers to foo.ts, a source file.

// Load JupyterLab:
await page.goto('http://localhost:8080/lab?reset');

// NOTE: depending on system resource constraints, this may NOT be enough time for JupyterLab to load and get "settled", so to speak. If CI tests begin inexplicably failing due to timeout failures, may want to consider increasing the sleep duration...
Copy link
Member

Choose a reason for hiding this comment

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

:D

// Based on from https://yarnpkg.com/en/package/@rws-air/jestscreenshot

const PuppeteerEnvironment = require("jest-environment-puppeteer");
const JestScreenshot = require("@rws-air/jestscreenshot");
require("jest-circus");
Copy link
Member

Choose a reason for hiding this comment

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

It's interesting you didn't need this! Not sure why we did, they just had it here.

Copy link
Member Author

Choose a reason for hiding this comment

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

This was present in one of the dep documentation examples. I am not sure why it was there. If it was needed, I would disappointed, as it means importing the package has (gasp) side-effects :|.

@saulshanabrook
Copy link
Member

I'm loving the green checkmark

@saulshanabrook saulshanabrook merged commit 8ecf944 into jupyterlab:testing Oct 24, 2019
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

2 participants