Skip to content

Fix browser tests #6870

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
ian-r-rose merged 8 commits into from
Jul 24, 2019
Merged

Fix browser tests #6870

ian-r-rose merged 8 commits into from
Jul 24, 2019

Conversation

@ian-r-rose
Copy link
Member

@ian-r-rose ian-r-rose commented Jul 22, 2019

Possible fix for CI woes in "Linux Usage" with notebook 6.0

References

jupyter/notebook#4260
Supersedes #6867

Code changes

  • Check for local login file in browser tests. If available, use that.
  • Wait for local login file to redirect when testing if the app loaded

User-facing changes

None

Backwards-incompatible changes

None

@ian-r-rose ian-r-rose added this to the 1.0.3 milestone Jul 22, 2019
@ian-r-rose ian-r-rose requested review from blink1073 and lresende July 22, 2019 20:32
@jupyterlab-dev-mode
Copy link

jupyterlab-dev-mode bot commented Jul 22, 2019

Thanks for making a pull request to JupyterLab!

To try out this branch on binder, follow this link: Binder

@ian-r-rose ian-r-rose mentioned this pull request Jul 22, 2019
Closed
blink1073
blink1073 approved these changes Jul 22, 2019
View reviewed changes
Copy link
Contributor

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

Thanks!

@ian-r-rose
Copy link
Member Author

ian-r-rose commented Jul 22, 2019 •
edited
Loading

Eep, this is still failing browser tests, just a bit further down the road.

@telamonian telamonian mentioned this pull request Jul 22, 2019
Merged
@ian-r-rose
Copy link
Member Author

ian-r-rose commented Jul 23, 2019

If anybody is able to discern why this is still failing for some examples (you can test from the examples directory by running python example_check.py console), that would be helpful -- I'm a bit stumped.

@telamonian
fixed waiting on local file redirect in chrome-example-test.js
af41dc1 
also added a `path` cmd line arg to `test_examples.py`
@telamonian
Copy link
Member

telamonian commented Jul 24, 2019 •
edited
Loading

@ian-r-rose You were definitely right about waiting for the redirect:

jupyterlab/examples/chrome-example-test.js

Lines 50 to 54 in fea72db

// Wait for the local file to redirect on noteboox >= 6.0,
// which happens after 1000ms.
await new Promise(resolve => {
setTimeout(resolve, 2000);
});

but apparently waiting for too long can lead to errors like:

Error: Protocol error (Runtime.callFunctionOn): Session closed. Most likely the page has been closed.

You can get the "goldilocks" wait like so:

await page.waitForNavigation();

Refs:
puppeteer/puppeteer#2696 (comment)
https://stackoverflow.com/q/46948489/425458
https://jupyter-notebook.readthedocs.io/en/stable/changelog.html?highlight=redirect#release-6-0

@telamonian
Copy link
Member

telamonian commented Jul 24, 2019

Ugggh, now the command line argument I added to test_examples.py is breaking things

ian-r-rose
ian-r-rose commented Jul 24, 2019
View reviewed changes
Copy link
Member Author

@ian-r-rose ian-r-rose left a comment

Choose a reason for hiding this comment

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

Thanks for finishing this off @telamonian!

@ian-r-rose ian-r-rose merged commit 63ef584 into jupyterlab:master Jul 24, 2019
@telamonian
Copy link
Member

telamonian commented Jul 24, 2019

No problem. Looking over the actual CI output logs, there are a ton of test errors that are being unhandled/uncaught for a variety of reasons. Example:

2019-07-24T02:35:23.3693041Z ++ rm -rf packages/foo
2019-07-24T02:35:23.3715816Z ++ jlpm run integrity
2019-07-24T02:35:24.6912331Z yarn run v1.15.2
2019-07-24T02:35:24.7349770Z $ node buildutils/lib/ensure-repo.js
2019-07-24T02:35:25.2088482Z (node:7972) UnhandledPromiseRejectionWarning: Error: Cannot find module 'foo/package.json'
2019-07-24T02:35:25.2089558Z     at Function.Module._resolveFilename (internal/modules/cjs/loader.js:636:15)
2019-07-24T02:35:25.2089806Z     at Function.resolve (internal/modules/cjs/helpers.js:33:19)
2019-07-24T02:35:25.2090041Z     at requirePackage (/home/vsts/work/1/s/buildutils/lib/utils.js:263:33)
2019-07-24T02:35:25.2090250Z     at Object.keys.forEach.depName (/home/vsts/work/1/s/buildutils/lib/utils.js:237:31)
2019-07-24T02:35:25.2090451Z     at Array.forEach (<anonymous>)
2019-07-24T02:35:25.2090635Z     at Object.keys.forEach.name (/home/vsts/work/1/s/buildutils/lib/utils.js:228:27)
2019-07-24T02:35:25.2090814Z     at Array.forEach (<anonymous>)
2019-07-24T02:35:25.2091008Z     at Object.getPackageGraph (/home/vsts/work/1/s/buildutils/lib/utils.js:224:25)
2019-07-24T02:35:25.2091673Z     at ensureIntegrity (/home/vsts/work/1/s/buildutils/lib/ensure-repo.js:227:25)
2019-07-24T02:35:25.2092449Z     at Object.<anonymous> (/home/vsts/work/1/s/buildutils/lib/ensure-repo.js:342:10)
2019-07-24T02:35:25.2094925Z (node:7972) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
2019-07-24T02:35:25.2097059Z (node:7972) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
2019-07-24T02:35:25.2219400Z Done in 0.54s.

Basically, integrity gets confused if you run it right after deleting a core package, but our related test (in the usage CI) doesn't error out, it just fails silently. At some point in the near future we should probably do a careful audit of all the tests.

@telamonian telamonian mentioned this pull request Jul 24, 2019
Merged
@lock lock bot added the status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Aug 23, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Aug 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@lresende lresende Awaiting requested review from lresende

1 more reviewer

@blink1073 blink1073 blink1073 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

maintenance status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. tag:Testing

Projects

None yet

Milestone

1.0.3

Development

Successfully merging this pull request may close these issues.

3 participants

@ian-r-rose @telamonian @blink1073