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 diff comments and cleanup #21032

Conversation

danielrozenberg
Copy link
Member

  • Added comments to visual-diff/index.js explaining what happens inside that giant function that was extremely opaque until now
  • Moved code-in-string to an external file
  • Removed unnecessary viewport resizing after wrapping the test inside an iframe
  • Doubled the number of parallel tabs. I tried various values and 20 seem to get the optimal result (more than that doesn't make things faster on my local machine, but on Travis it might be different... I could experiment with this more if we start seeing a surge in test numbers ,but for now it runs in ~1m and that's fine)
  • Simpler viewport sizing logic
  • Renamed loading_[in]complete_css => loading_[in]complete_selectors (because they are css selectors, not css code)
  • Added detailed explanation on order of execution of tests to visual-tests file

@@ -127,21 +127,42 @@
* // snapshot on Percy that demonstrate the flakiness of this test.
* "flaky": true
* },
*
* Each webpages (or, optionally, each of its tests in the interactive_tests
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: "webpage"

* performing these tasks in order:
* - Load the page and waits for network activity to stop (or up to 3 secs)
* - Wait for the built-in AMP loader dots to disappear from the page, meaning
* all AMP components finished being layed out and all resources, such as
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: "laid"

@danielrozenberg danielrozenberg merged commit 0031b01 into ampproject:master Feb 22, 2019
@danielrozenberg danielrozenberg deleted the visual-diff-comments-and-cleanup branch February 22, 2019 23:26
noranazmy pushed a commit to noranazmy/amphtml that referenced this pull request Mar 22, 2019
* visual-diff comments, explanation, and minor cleanup

* Make the viewport resizing more clear

* Double the number of parallel tabs

* Fixed typos
bramanudom pushed a commit to bramanudom/amphtml that referenced this pull request Mar 22, 2019
* visual-diff comments, explanation, and minor cleanup

* Make the viewport resizing more clear

* Double the number of parallel tabs

* Fixed typos
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants