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

AMP validation for all scenarios #442

Closed
sareh opened this issue Aug 6, 2018 · 4 comments
Closed

AMP validation for all scenarios #442

sareh opened this issue Aug 6, 2018 · 4 comments
Labels
AMP Work related to AMP blocked This issue should not be worked on until another internal issue is completed - see desc for details Refinement Needed This is ready for refinement. It shouldn't be worked on until it has been refined by Dev & Test.
Milestone

Comments

@sareh
Copy link
Contributor

sareh commented Aug 6, 2018

Is your feature request related to a problem? Please describe.
After steps #440 (creating amp routes) and #441 (validating a single scenario) have been completed, we want to run the AMP validation script against all article scenarios.

Describe the solution you'd like
Update the amp validation npm script to run against all of the article scenarios and output their results in the console.

If there are any errors listed in the output, a separate issue should be opened to create an AMP-compliant component & container that addresses those points.

Additional context
See instructions here https://www.ampproject.org/docs/fundamentals/validate#command-line-tool and here https://www.npmjs.com/package/amphtml-validator

@sareh sareh added the blocked This issue should not be worked on until another internal issue is completed - see desc for details label Aug 6, 2018
@sareh sareh added this to the Application Setup milestone Aug 6, 2018
@chris-hinds
Copy link
Contributor

Same question as #441 really, is to this run locally or on CI?

We have an AMP validator that runs on a Lambda and is validates every single published AMP article, posting to Slack with the error's (if there are any) if you are thinking of this as more of an end to end test maybe using the same validator setup we are now would be a good idea?

The current validator runs on both test and live.

@jtart jtart added the AMP Work related to AMP label Aug 9, 2018
@chris-hinds chris-hinds self-assigned this Aug 10, 2018
@sareh sareh reopened this Sep 4, 2018
@sareh sareh removed the blocked This issue should not be worked on until another internal issue is completed - see desc for details label Sep 10, 2018
@radiocontrolled radiocontrolled added the blocked This issue should not be worked on until another internal issue is completed - see desc for details label Oct 30, 2018
@radiocontrolled
Copy link
Contributor

Blocked pending #873

@hindsc52 in the future when all the validation errors are sorted we will want to run this in CI but at present it would break CI.

@ChrisBAshton
Copy link
Contributor

Related to #1122.

@sareh sareh added Refinement Needed This is ready for refinement. It shouldn't be worked on until it has been refined by Dev & Test. simorgh-core-stream labels Apr 9, 2019
@chris-hinds chris-hinds removed their assignment Apr 9, 2019
@jamesbhobbs
Copy link
Contributor

I don't believe we need to validate every article, but this #2220 is very valuable and quite similar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AMP Work related to AMP blocked This issue should not be worked on until another internal issue is completed - see desc for details Refinement Needed This is ready for refinement. It shouldn't be worked on until it has been refined by Dev & Test.
Projects
None yet
Development

No branches or pull requests

6 participants