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

Validation tests for examples and test fixtures #25149

Closed
jpettitt opened this issue Oct 18, 2019 · 6 comments
Closed

Validation tests for examples and test fixtures #25149

jpettitt opened this issue Oct 18, 2019 · 6 comments
Assignees
Labels
P2: Soon Type: Feature Request WG: Caching: Triaged Initial triage from wg-caching complete. Remove label if new input required. WG: infra

Comments

@jpettitt
Copy link
Contributor

amphtml files in /examples and /test/fixtures/e2e should be validated.

Errors in test pages can cause test flakes (see #25145). Having these files validated (if only as part of the local changes check) would help catch these hard to debug flakes.

@rsimha
Copy link
Contributor

rsimha commented Oct 18, 2019

We currently run a whole host of validator checks via gulp validator. Would it be possible to add our test html fixtures to the list of files that are validated?

@honeybadgerdontcare @Gregable Thoughts?

/cc @ampproject/wg-caching

@honeybadgerdontcare
Copy link
Contributor

honeybadgerdontcare commented Oct 18, 2019

The tests we have for the validator are explicitly to test validation scenarios (both PASS and FAIL). It sounds like the request here is to make sure that example files can only be valid AMP (PASS). Is that a correct reading?

If so, we could have a test that won't allow merging unless all files in certain directories pass validation. However, would this impede work that is in progress but not yet valid?

Are there other directories this could apply to other than /examples/.html and /test/fixtures/e2e/.html?

@cramforce
Copy link
Member

We should do it, but allow for allow-listing expected failures.

@Gregable
Copy link
Member

@rsimha I feel this is an infra feature request, not validation. Feel free to re-label if you disagree.

@jpettitt
Copy link
Contributor Author

Yes the intent was that people making examples should make valid ones.

@Gregable Gregable added the WG: Caching: Triaged Initial triage from wg-caching complete. Remove label if new input required. label Feb 11, 2020
@rsimha rsimha self-assigned this Apr 22, 2021
@rsimha
Copy link
Contributor

rsimha commented Apr 30, 2021

The primary feature requested by this issue has been implemented. Existing invalid fixtures can be fixed piecemeal by following the instructions at #34000 (comment).

@rsimha rsimha closed this as completed Apr 30, 2021
alanorozco added a commit that referenced this issue Jun 15, 2021
Partial for #25149.

1. Fixes a set of documents.
2. Removes unused example files that do not validate.
3. Categorizes some ignore rules that cannot be removed.
4. Adds a general rule for `bento/` directories, for documents which are not meant to be valid.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2: Soon Type: Feature Request WG: Caching: Triaged Initial triage from wg-caching complete. Remove label if new input required. WG: infra
Projects
None yet
Development

No branches or pull requests

6 participants