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

🏗✨✅ Introduce interactivity to visual diff tests (experimental) #19114

Conversation

danielrozenberg
Copy link
Member

Adds an interactive_tests field to the visual-tests JSON5 file.

The field is a file path to a JavaScript file that exports a dictionary of (async) test functions keyed by a short descriptive name.

Each test function is passed two arguments: a reference to the Puppeteer page, and the full name of the test (page name + test name). e.g., the JS file can be:

module.exports = {
  'tap red button': async (page, name) => {
    await page.tap('#red_button');
    await verifyCssElements(page, name, null, ['.red-loading'],
      ['.red-loaded']);
  },
  'tap red and blue button': async (page, name) => {
    await page.tap('#red_button');
    await page.tap('#blue_button');   
    await verifyCssElements(page, name, null,
      ['.red-loading', '.blue-loading'],
      ['.red-loaded', '.blue-loaded']);
  },
};

This PR also adds a single test as a proof-of-concept :)

@danielrozenberg
Copy link
Member Author

/cc @newmuis - you might be interested in this

@newmuis
Copy link
Contributor

newmuis commented Nov 3, 2018

I am indeed very interested in this.

* // reference to the Puppeteer page, and the full name of the test (page
* // name + test name). e.g., the JS file can be:
* // module.exports = {
* // 'tap red button': async (page, name) => {
Copy link
Member

Choose a reason for hiding this comment

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

is the key here sort of just a description? it does nothing as far as execution correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a description, but it also sets the name of the snapshot in Percy - and that determines what snapshot any future PR will be compared against

e.g., in the example test that I added, I named the test tap "see more" button, and in Percy the name of the snapshot is AMP List and Mustache (tap "see more" button)
https://percy.io/ampproject/amphtml/builds/1137857

// load those tests here.
for (const webpage of webpages) {
webpage.tests_ = {
'': async() => {},
Copy link
Member

Choose a reason for hiding this comment

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

whats the empty func for?

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sure each page has a "zero interactions" test as well (it's easier to add this empty function to the list of tests than it is to create a special case before the iterator later on)

Copy link
Contributor

@rsimha rsimha left a comment

Choose a reason for hiding this comment

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

Excited to see this happen!

build-system/tasks/visual-diff/index.js Show resolved Hide resolved
@danielrozenberg danielrozenberg merged commit 67f17dd into ampproject:master Nov 8, 2018
@danielrozenberg danielrozenberg deleted the visual-diff-interactive-prototype branch November 8, 2018 21:24
@newmuis newmuis mentioned this pull request Nov 19, 2018
Enriqe pushed a commit to Enriqe/amphtml that referenced this pull request Nov 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants