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

Implement infrastructure for golden file pixel-diff rendering tests #8

Closed
cramforce opened this issue Sep 3, 2015 · 7 comments
Closed
Assignees
Milestone

Comments

@cramforce
Copy link
Member

@dvoytenko hacked something up based on phantom.

We should make this so that you can easily add samples and confirm that rendering stays the same as you make changes (with the option to approve diffs, of course)

@rudygalfi
Copy link
Contributor

rudygalfi commented Feb 17, 2017

@erwinmombay Assigning this to you as the best possible assignee. Please re-assign if needed or close.

@rsimha
Copy link
Contributor

rsimha commented May 18, 2017

Basic infrastructure is in place. Tracking the addition of more tests via separate issues.

@cramforce
Copy link
Member Author

cramforce commented May 19, 2017

I have a few questions that I'm not clear on:

  • Are we blocking PRs yet on these tests succeeding?
  • Can everyone see the diffs? I saw something about redacted secret tokens in the URL.

@rsimha
Copy link
Contributor

rsimha commented May 22, 2017

  • We are not blocking PRs yet. I'm tracking that work via Make visual diff tests PR blocking in case of failures #9469.
  • The free version of Percy doesn't appear to allow everyone to see the diffs. This has nothing to do with secret tokens (that's related to logs). I'm in the process of exploring a paid account on Percy to see if that will give us the ability to make diffs public.

@cramforce
Copy link
Member Author

Did you make progress on "Can everyone see the diffs?"

I think they require an invite, right?

@rsimha
Copy link
Contributor

rsimha commented Jun 19, 2017

@cramforce, I've invited all internal commiters to view the diffs, so they should have access. Still exploring the possibility of giving blanket access to anyone logged in with a github account who also has access to the travis logs on ampproject/amphtml.

@cramforce
Copy link
Member Author

@rsimha-amp Ideally they would be public. Logged in with GH account would be OK. As liberal as we can be.

alabiaga pushed a commit that referenced this issue Feb 28, 2019
* creating new amp-smartlinks extension

* BAM-2584 AMP-Smartlinks (#3)

* adding example page and amp-smartlinks to bundles.config

* creating amp-smartlinks scaffolding

* BAM-2584 adding hardcoded POC implementation of amp-smartlinks

* adding new constants file and linkmate file/class

* adding linkmate call and association working with link-rewrite service

* add amp config call and update workflow to use those values

* setup initial amp-smartlinks and linkmate workflow

* adding new config variables for link attribute and selector

* add tests for amp-smartlinks

* add test for linkmate.js

* add more thorough anchorList check in runSmartlinks

* adding code comments, new constants structure and better options validation

* update amp-smartlinks to have helpful information

* updating global vars and cleaning up main files

* clean up tests and add more explicit type assertions

* clean up jsdoc tags

* BAM-2585 Whitelist import, fix type errors, and replace user.assert with userAssert (#4)

* BAM-2585 whitelist navigation import and fix type errors

* replace user.assert with userAssert

* BAM-2585 fix validator and type check (#5)

* more validator fixes (#6)

* BAM-2585 Fix validator, copyright, and whitespace (#7)

* alphabetize validator, fix whitespace, and add valid tag

* update year in copyright statement

* BAM-2585 move `link-rewriter` to import statements and updating types (#8)

* BAM-2585 remove link-rewriter and switch to importing from skimlinks extension

* clean up promise chain and more descriptive API comments

* update xhr to pull from ampdoc.win and add types to constants.js

* fix type in buildPageImpressionPayload_

* update validator with new empty value check (#9)

* update validator for exclusive-links

* switch page-impression API request to customEventReporter (#11)

* BAM-2585 fix jsdoc in linkmate-options and unnecessary param in page_impression request (#12)

* fix jsdoc for linkmate params and make runLinkmate more readable (#13)

* add try/catch on amp_config fetch and updated constants.js

* remove bad type in constants.js and add check for existing shop-links (#14)

* add check for auction_id in mapLinks and add jsdoc for SMARTLINKS_REWRITER_ID

* fix type notation in constants.js and linkmate-options.js

* fix indentation in example file

* add note to README describing link-rewriter priority queue behavior (#15)

* add exception to compile.js for amp-skimlinks

* update validator to use empty value as indicator linkmate param

* fix validator and linkmate-options to use new config style (#16)

* updating tests for linkmate-options.js

* remove redundant userAssert in linkmate-options

* update tests to reflect config changes

* update tests to send accurate config params

* update readme to refelct config changes and more accurate function names in linkmate-options
noranazmy pushed a commit to noranazmy/amphtml that referenced this issue Mar 22, 2019
* creating new amp-smartlinks extension

* BAM-2584 AMP-Smartlinks (#3)

* adding example page and amp-smartlinks to bundles.config

* creating amp-smartlinks scaffolding

* BAM-2584 adding hardcoded POC implementation of amp-smartlinks

* adding new constants file and linkmate file/class

* adding linkmate call and association working with link-rewrite service

* add amp config call and update workflow to use those values

* setup initial amp-smartlinks and linkmate workflow

* adding new config variables for link attribute and selector

* add tests for amp-smartlinks

* add test for linkmate.js

* add more thorough anchorList check in runSmartlinks

* adding code comments, new constants structure and better options validation

* update amp-smartlinks to have helpful information

* updating global vars and cleaning up main files

* clean up tests and add more explicit type assertions

* clean up jsdoc tags

* BAM-2585 Whitelist import, fix type errors, and replace user.assert with userAssert (ampproject#4)

* BAM-2585 whitelist navigation import and fix type errors

* replace user.assert with userAssert

* BAM-2585 fix validator and type check (ampproject#5)

* more validator fixes (ampproject#6)

* BAM-2585 Fix validator, copyright, and whitespace (ampproject#7)

* alphabetize validator, fix whitespace, and add valid tag

* update year in copyright statement

* BAM-2585 move `link-rewriter` to import statements and updating types (ampproject#8)

* BAM-2585 remove link-rewriter and switch to importing from skimlinks extension

* clean up promise chain and more descriptive API comments

* update xhr to pull from ampdoc.win and add types to constants.js

* fix type in buildPageImpressionPayload_

* update validator with new empty value check (ampproject#9)

* update validator for exclusive-links

* switch page-impression API request to customEventReporter (ampproject#11)

* BAM-2585 fix jsdoc in linkmate-options and unnecessary param in page_impression request (ampproject#12)

* fix jsdoc for linkmate params and make runLinkmate more readable (ampproject#13)

* add try/catch on amp_config fetch and updated constants.js

* remove bad type in constants.js and add check for existing shop-links (ampproject#14)

* add check for auction_id in mapLinks and add jsdoc for SMARTLINKS_REWRITER_ID

* fix type notation in constants.js and linkmate-options.js

* fix indentation in example file

* add note to README describing link-rewriter priority queue behavior (ampproject#15)

* add exception to compile.js for amp-skimlinks

* update validator to use empty value as indicator linkmate param

* fix validator and linkmate-options to use new config style (ampproject#16)

* updating tests for linkmate-options.js

* remove redundant userAssert in linkmate-options

* update tests to reflect config changes

* update tests to send accurate config params

* update readme to refelct config changes and more accurate function names in linkmate-options
bramanudom pushed a commit to bramanudom/amphtml that referenced this issue Mar 22, 2019
* creating new amp-smartlinks extension

* BAM-2584 AMP-Smartlinks (ampproject#3)

* adding example page and amp-smartlinks to bundles.config

* creating amp-smartlinks scaffolding

* BAM-2584 adding hardcoded POC implementation of amp-smartlinks

* adding new constants file and linkmate file/class

* adding linkmate call and association working with link-rewrite service

* add amp config call and update workflow to use those values

* setup initial amp-smartlinks and linkmate workflow

* adding new config variables for link attribute and selector

* add tests for amp-smartlinks

* add test for linkmate.js

* add more thorough anchorList check in runSmartlinks

* adding code comments, new constants structure and better options validation

* update amp-smartlinks to have helpful information

* updating global vars and cleaning up main files

* clean up tests and add more explicit type assertions

* clean up jsdoc tags

* BAM-2585 Whitelist import, fix type errors, and replace user.assert with userAssert (ampproject#4)

* BAM-2585 whitelist navigation import and fix type errors

* replace user.assert with userAssert

* BAM-2585 fix validator and type check (ampproject#5)

* more validator fixes (ampproject#6)

* BAM-2585 Fix validator, copyright, and whitespace (ampproject#7)

* alphabetize validator, fix whitespace, and add valid tag

* update year in copyright statement

* BAM-2585 move `link-rewriter` to import statements and updating types (ampproject#8)

* BAM-2585 remove link-rewriter and switch to importing from skimlinks extension

* clean up promise chain and more descriptive API comments

* update xhr to pull from ampdoc.win and add types to constants.js

* fix type in buildPageImpressionPayload_

* update validator with new empty value check (ampproject#9)

* update validator for exclusive-links

* switch page-impression API request to customEventReporter (ampproject#11)

* BAM-2585 fix jsdoc in linkmate-options and unnecessary param in page_impression request (ampproject#12)

* fix jsdoc for linkmate params and make runLinkmate more readable (ampproject#13)

* add try/catch on amp_config fetch and updated constants.js

* remove bad type in constants.js and add check for existing shop-links (ampproject#14)

* add check for auction_id in mapLinks and add jsdoc for SMARTLINKS_REWRITER_ID

* fix type notation in constants.js and linkmate-options.js

* fix indentation in example file

* add note to README describing link-rewriter priority queue behavior (ampproject#15)

* add exception to compile.js for amp-skimlinks

* update validator to use empty value as indicator linkmate param

* fix validator and linkmate-options to use new config style (ampproject#16)

* updating tests for linkmate-options.js

* remove redundant userAssert in linkmate-options

* update tests to reflect config changes

* update tests to send accurate config params

* update readme to refelct config changes and more accurate function names in linkmate-options
developit added a commit to developit/amphtml that referenced this issue Sep 16, 2019
Fix builds by upgrading to Node 12 and ignoring strange write failures.
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

5 participants