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

🏗 Update @percy/percy-puppeteer to v1.x #25176

Merged
merged 5 commits into from Oct 22, 2019

Conversation

danielrozenberg
Copy link
Member

@danielrozenberg danielrozenberg commented Oct 21, 2019

Fixes #24869

This PR updates the @percy/percy-puppeteer dependency from v0.4.0 to v1.0.8, and also:

Major:

  • Executes the @percy/agent binary in a subprocess of the gulp task (the agent is required by the updated dependency)
  • Removes the special-case for Travis logs (running series of dots) and display each test start/end on Travis
    • This was done because the new Percy agent is verbose on its own, and doesn't have an easy way to silence it. Since the visual diff tests are now running on their own separate shard, this doesn't spam the logs
    • To display verbose logs locally, pass --verbose to the task

Minor:

  • Removes the custom assets loader, since assets are handled by the updated dependency
  • Removes unnecessary .gitkeep file for a no-longer blank directory
  • Replaced default shell for spawned processes on Unix from /bin/sh to /bin/bash per Can't kill child process travis-ci/travis-ci#704 (comment)

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.

Really excited to see this happen! Some initial comments below.

build-system/tasks/visual-diff/index.js Show resolved Hide resolved
build-system/tasks/visual-diff/index.js Show resolved Hide resolved
build-system/tasks/visual-diff/index.js Show resolved Hide resolved
build-system/tasks/visual-diff/index.js Show resolved Hide resolved
build-system/tasks/visual-diff/index.js Show resolved Hide resolved
build-system/tasks/visual-diff/package.json Show resolved Hide resolved
build-system/tasks/visual-diff/index.js Show resolved Hide resolved
build-system/tasks/visual-diff/index.js Show resolved Hide resolved
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.

Checked out this branch on a Macbook and sanity tested commands like gulp dist and gulp check-types, both of which use execScriptAsync under the covers. Everything looks good so far. Thanks for doing this!

@danielrozenberg danielrozenberg merged commit 7a0a66f into ampproject:master Oct 22, 2019
@danielrozenberg danielrozenberg deleted the percy-puppeteer branch October 22, 2019 20:37
joshuarrrr pushed a commit to Parsely/amphtml that referenced this pull request Oct 22, 2019
* Update @percy/percy-puppeteer to v1.x

* Removed no-longer used lines from .gitignore

* Await until all subprocesses exit

* Replace command shell on Unix from `/bin/sh` to `/bin/bash`

* Change how child processes are spawn to align with child_process.spawn docs
estherkim added a commit that referenced this pull request Oct 22, 2019
estherkim added a commit that referenced this pull request Oct 23, 2019
danielrozenberg added a commit to danielrozenberg/amphtml that referenced this pull request Oct 23, 2019
danielrozenberg added a commit that referenced this pull request Oct 23, 2019
* Revert "Revert "🏗 Update @percy/percy-puppeteer to v1.x (#25176)" (#25213)"

This reverts commit d134d30.

* Do not expect a build when running with --empty
joshuarrrr pushed a commit to Parsely/amphtml that referenced this pull request Oct 23, 2019
joshuarrrr pushed a commit to Parsely/amphtml that referenced this pull request Oct 23, 2019
…ampproject#25221)

* Revert "Revert "🏗 Update @percy/percy-puppeteer to v1.x (ampproject#25176)" (ampproject#25213)"

This reverts commit d134d30.

* Do not expect a build when running with --empty
micajuine-ho pushed a commit to micajuine-ho/amphtml that referenced this pull request Dec 27, 2019
* Update @percy/percy-puppeteer to v1.x

* Removed no-longer used lines from .gitignore

* Await until all subprocesses exit

* Replace command shell on Unix from `/bin/sh` to `/bin/bash`

* Change how child processes are spawn to align with child_process.spawn docs
micajuine-ho pushed a commit to micajuine-ho/amphtml that referenced this pull request Dec 27, 2019
micajuine-ho pushed a commit to micajuine-ho/amphtml that referenced this pull request Dec 27, 2019
…ampproject#25221)

* Revert "Revert "🏗 Update @percy/percy-puppeteer to v1.x (ampproject#25176)" (ampproject#25213)"

This reverts commit d134d30.

* Do not expect a build when running with --empty
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.

Upgrade @percy/puppeteer to v1.x
4 participants