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

Percy correctly loads image assets, which then do not appear in snapshots #10156

Closed
rsimha opened this issue Jun 27, 2017 · 4 comments
Closed
Assignees

Comments

@rsimha
Copy link
Contributor

rsimha commented Jun 27, 2017

Here is the test log corresponding to Percy build https://percy.io/ampproject/amphtml/builds/270874. Note that the image assets are being loaded and fetched, but they don't appear in the snapshots.

ruby build-system/tasks/visual-diff.rb 
[11:58:39] Using gulpfile ~/src/amphtml/gulpfile.js
[11:58:39] Starting 'serve'...
[11:58:39] Serving unminified js
[11:58:39] Run `gulp build` then go to http://localhost:8000/examples/article.amp.html
[11:58:39] Finished 'serve' after 2.77 ms
[11:58:40] Webserver started at http://localhost:8000
HEAD / 200 21.118 ms - 12679
HEAD / 200 4.149 ms - 12679
[percy][DEBUG] Using filesystem_loader to discover assets.
[percy][DEBUG] Build resource: /amp-by-example/amp-by-example.html
[percy][DEBUG] Build resource: /amp-by-example/amp_by_example_logo.svg
[percy][DEBUG] Build resource: /article.amp/img/sea@1x.jpg
[percy][DEBUG] Build resource: /article.amp/img/hero@1x.jpg
[percy][DEBUG] Build resource: /article.amp/img/sample.jpg
[percy][DEBUG] Build resource: /article.amp/img/hero@2x.jpg
[percy][DEBUG] Build resource: /article.amp/img/sea@2x.jpg
[percy][DEBUG] Build resource: /article.amp/article.amp.html
[percy][DEBUG] All build resources loaded (0.009959472s)
[percy][DEBUG] Build 270874 created
[percy][DEBUG] Parallel test environment: not detected
GET /examples/visual-tests/amp-by-example/amp-by-example.html 200 14.908 ms - 116573
GET /dist/v0/amp-selector-0.1.max.js 200 11.441 ms - 189182
GET /dist/v0/amp-sidebar-0.1.max.js 200 12.175 ms - 208400
GET /dist/v0/amp-install-serviceworker-0.1.max.js 200 10.986 ms - 206187
GET /dist/v0/amp-analytics-0.1.max.js 200 10.971 ms - 496751
GET /dist/v0/amp-accordion-0.1.max.js 200 2.965 ms - 106968
GET /dist/amp.js 200 9.461 ms - 1185023
Powered by AMP ⚡ HTML – Version 1498511723360 http://localhost:8000/examples/visual-tests/amp-by-example/amp-by-example.html
GET /examples/visual-tests/amp-by-example/amp_by_example_logo.svg 200 0.907 ms - 1876
[percy][DEBUG] Using filesystem_loader to discover assets.
[percy][DEBUG] Snapshot started (name: "Amp By Example")
[percy][DEBUG] Snapshot resource: /examples/visual-tests/amp-by-example/amp-by-example.html
[percy][DEBUG] All snapshot resources loaded (0.044213695s)
[percy][DEBUG] All snapshot resources uploaded (0.171891766s)
GET /examples/visual-tests/article.amp/article.amp.html 200 4.569 ms - 14144
GET /dist/v0/amp-lightbox-0.1.max.js 200 8.583 ms - 248652
GET /dist/v0/amp-sidebar-0.1.max.js 200 10.370 ms - 208400
GET /dist/v0/amp-app-banner-0.1.max.js 200 8.361 ms - 212754
GET /dist/v0/amp-analytics-0.1.max.js 200 8.422 ms - 496751
GET /dist/v0/amp-ad-0.1.max.js 200 7.915 ms - 676432
GET /dist/amp.js 200 1.672 ms - 1185023
Powered by AMP ⚡ HTML – Version 1498511723360 http://localhost:8000/examples/visual-tests/article.amp/article.amp.html
GET /examples/visual-tests/article.amp/img/hero@1x.jpg 200 0.966 ms - 79160
GET /examples/visual-tests/article.amp/img/sea@1x.jpg 200 2.300 ms - 63914
GET /examples/visual-tests/article.amp/img/sample.jpg 200 2.413 ms - 28505
[percy][DEBUG] Using filesystem_loader to discover assets.
[percy][DEBUG] Snapshot started (name: "AMP Article")
[percy][DEBUG] Snapshot resource: /examples/visual-tests/article.amp/article.amp.html
[percy][DEBUG] All snapshot resources loaded (0.00297421s)
[percy][DEBUG] All snapshot resources uploaded (0.146531456s)

[percy] Visual diffs are now processing: https://percy.io/ampproject/amphtml/builds/270874

Done! Percy snapshots are now processing...
--> https://percy.io/ampproject/amphtml/builds/270874
[11:58:43] Shutting down server

@rsimha rsimha added this to the Sprint H2 June milestone Jun 27, 2017
@rsimha rsimha self-assigned this Jun 27, 2017
@rsimha
Copy link
Contributor Author

rsimha commented Jun 27, 2017

This might need additional options or command line args. See https://github.com/teampoltergeist/poltergeist and http://phantomjs.org/api/command-line.html

@cramforce
Copy link
Member

Copying over:

"How about injecting a custom JS binary into every page we diff that "documents" its progress with CSS classes. That would make waiting for onload trivial and allow for waiting for fonts to load."

rsimha added a commit that referenced this issue Jun 27, 2017
This PR does the following:

Reorganizes the code in visual-diff.rb to support multiple webpages in one Percy build
Reorganizes the directory structure of examples/visual-tests to support the upload of test assets in one fell swoop (Percy doesn't support piecemeal asset uploading)
Adds a new visual test
Makes progress on #10155
Partially blocked by #10156
@rsimha
Copy link
Contributor Author

rsimha commented Jun 27, 2017

After experimenting with various command line arguments to phantomjs, I'm almost certain that this is due to the fact that there's currently no signal that an AMP page has fully loaded its assets. Will now pursue Malte's suggestion above.

@rsimha
Copy link
Contributor Author

rsimha commented Jul 6, 2017

After investigating this further, waiting for a while between page.visit and Percy::Capybara.snapshot doesn't seem to be making any difference. The images are still missing. However, the webserver logs do indicate that the assets are being loaded after the call to page.visit.

I'll raise this with the Percy team via a bug in their repository.

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

No branches or pull requests

3 participants