add module build visual diffs

[erwinmombay]

add a module build visual diff job using the gulp serve --new_server task which does a transform of the documents

[amp-owners-bot[bot]]

Hey @rsimha! These files were changed:

.circleci/config.yml
build-system/pr-check/visual-diff-tests.js

Hey @danielrozenberg! These files were changed:

build-system/tasks/visual-diff/index.js

[danielrozenberg]

I think a more maintainable way to do this is to make the job script visual-diff-tests.js take a command line arg, and parametrize the job definition in config.yml.

#32808 has an example of how to do it

[danielrozenberg]

Since both jobs use the same Percy write-only token, they'll both end up in the same project, and Percy will only report one as a status to GitHub - whichever of the two ends later would override the earlier.

Two possible solutions to this are:

1. create separate projects (ampproject/amphtml-module and ampproject/amphtml-nomodule) with their own tokens, passed as parameters in the job matrix
2. merge the two by running them in a single job by running the same test suite twice, on two executions of the server (with/without esm) and with adding (module)/(nomodule) to the individual snapshot names to differentiate between the two

I am very slightly leaning towards option #1, but don't really care either way. @rsimha can you think of how to pass the obfuscated token to the PR jobs?

[rsimha]

Since both jobs use the same Percy write-only token, they'll both end up in the same project, and Percy will only report one as a status to GitHub - whichever of the two ends later would override the earlier.

Oooh, excellent point, and good catch! TBH, after looking at the build logs on CircleCI and on Percy, I am leaning hard towards option 2. Here is my rationale:

• When you look at a Percy build for a single commit, it's useful to see all results (from module and nomodule) on one page. This way, if module breaks something but nomodule does not, it will be very obvious.
• Finalizing a visual build for a given commit from one CI job once and for all seems far simpler than trying to coordinate across two jobs, not to mention the cost of maintaining two tokens and two projects.
• Looking at the actual cost of parallelizing on CircleCI, VM setup / install takes ~2m, and running of all visual tests takes ~2m. I think it's better to test both build types from one job that runs for ~6m. (I assume it will work if we make the job wait for both the module and nomodule builds, download both, and run the correct kind of test server for each testing run.)

WDYT @erwinmombay @danielrozenberg? Maybe a little extra trouble up front to get this to work, but worth it in the end?

[danielrozenberg]

Again, no strong feelings either way. The overhead having two tokens/projects isn't that serious though, and we already discussed having separate projects for Chrome and for Firefox once Puppeteer fully supports the latter

[rsimha]

Again, no strong feelings either way. The overhead having two tokens/projects isn't that serious though, and we already discussed having separate projects for Chrome and for Firefox once Puppeteer fully supports the latter

Okay, in that case, I'll lower my hard preference to a mild preference, and let @erwinmombay chime in since he's going to be the primary user of the Percy builds. 😃

Re: adding new tokens to our CircleCI project, it's no trouble, and I can help do it. 👍

[erwinmombay]

@rsimha i think i would prefer option 2, just because seeing the results in a single page seems ideal

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.