Skip to content

Conversation

@jtpio
Copy link
Member

@jtpio jtpio commented Sep 7, 2023

References

The trace option was added as a comment in #13909, but #13909 does not mention why it was left as a comment.

Playwright docs: https://playwright.dev/docs/trace-viewer

Code changes

Enable trace so it's easier to debug failing UI tests on CI.

User-facing changes

None

Backwards-incompatible changes

None

@jupyterlab-probot
Copy link

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link: Binder

@jtpio
Copy link
Member Author

jtpio commented Sep 7, 2023

Since the UI tests are failing on this PR (#14947) it's a good occasion to show what it looks like when opening the Playwright report:

playwright-trace-demo.mp4

@jtpio
Copy link
Member Author

jtpio commented Sep 8, 2023

One downside is that it might increase the size of the artifacts when many tests are failing:

image

@krassowski
Copy link
Member

30 MB is not that bad, I think it is worth it.

I was also considering whether to enable console log forwarding as in: krassowski@8e5cb06, which has a downside of making the progress a bit more noisy but then we probably should take care not to spam with logs anyways so maybe this is a feature not an issue - thoughts?

@fcollonval
Copy link
Member

30 MB is not that bad, I think it is worth it.

👍

I was also considering whether to enable console log forwarding as in: krassowski@8e5cb06, which has a downside of making the progress a bit more noisy but then we probably should take care not to spam with logs anyways so maybe this is a feature not an issue - thoughts?

We could add a configurable fixture for that to get the console to be forwarded only on retry on the CI for example. But that could be changed locally for debug.

Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

Thanks @jtpio

@jtpio
Copy link
Member Author

jtpio commented Sep 12, 2023

30 MB is not that bad, I think it is worth it.

Right for this run it's fine. But I think it will be proportional to the number of failing tests.

@jtpio jtpio merged commit f2a9603 into jupyterlab:main Sep 13, 2023
@jtpio jtpio deleted the playwright-trace branch September 13, 2023 08:22
@jtpio jtpio added this to the 4.0.x milestone Sep 13, 2023
@jtpio
Copy link
Member Author

jtpio commented Sep 13, 2023

@meeseeksdev please backport to 4.0.x

meeseeksmachine pushed a commit to meeseeksmachine/jupyterlab that referenced this pull request Sep 13, 2023
krassowski pushed a commit that referenced this pull request Sep 13, 2023
Co-authored-by: Jeremy Tuloup <jeremy.tuloup@gmail.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants