Skip to content
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.

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

Visual regression testing using Galata #3172

Merged
merged 16 commits into from
Mar 31, 2021
Merged

Conversation

mbektas
Copy link
Member

@mbektas mbektas commented Mar 25, 2021

  • adds visual regression testing support using https://github.com/jupyterlab/galata
  • test suite uploads and runs a notebook, that has code cells creating different widget instances, in JupyterLab using headless browser
  • cell output image captures are taken and compared to reference images
  • test output artifacts are saved and accessible on workflow page

(mbektas#4 demonstrates detecting visual regression caused by change to widget label rendering)

@ellisonbg @blink1073 @afshin FYI, this is the first integration of Galata in a Jupyter project! More to come.

@mbektas mbektas requested a review from jasongrout March 25, 2021 20:24
@github-actions
Copy link

Binder 👈 Launch a binder notebook on branch mbektasbbg/ipywidgets/galata

@mbektas mbektas requested a review from jtpio March 25, 2021 20:24
@mbektas mbektas changed the title Automated visual regression testing using Galata Visual regression testing using Galata Mar 25, 2021
@mbektas
Copy link
Member Author

mbektas commented Mar 25, 2021

Sample UI test result report generated as artifact
ipywidgets galata report

and diff related to failed test

widgets_cell_0

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.

Love it!

@jtpio
Copy link
Member

jtpio commented Mar 26, 2021

Looks great, thanks @mbektasbbg for this!

@@ -0,0 +1,16 @@
{
"name": "ipywidgets-ui-tests",
Copy link
Member

Choose a reason for hiding this comment

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

Wondering whether this new package should be added to the existing lerna packages? Then we might be able to do without ui-tests/package-lock.json and keep dependencies in a top-level yarn.lock.

Or is it better to keep the ui-tests package a bit more separated from the rest?

Copy link
Member Author

Choose a reason for hiding this comment

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

@jtpio good point. Initially I tried adding as dependency to top level package.json. I had some build issues then since rest of the project is using chai for unit testing and that was causing name clashes with jest that Galata uses (names like it, test, expect). Also, having a separate ui-tests directory & package decouples things nicely. Only the developers that add UI tests need to install the dependencies (which include browser binaries) and UI tests can work with different jupyterlab versions than ipywidgets is dependent on.

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 will add some documentation to guide on adding new UI test cases, building and testing locally.

Copy link
Member

Choose a reason for hiding this comment

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

Ah right, there can indeed be some clashes (also since galata is built for a specific version of lab).

.github/workflows/tests.yml Outdated Show resolved Hide resolved
Copy link
Member

@jtpio jtpio 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 merged commit 624dfc8 into jupyter-widgets:master Mar 31, 2021
@jtpio jtpio added this to the 8.0 milestone Mar 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants