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

Add configuration parsing to transformer spec test runner #4654

Closed
schlessera opened this issue May 5, 2020 · 2 comments · Fixed by #4662
Closed

Add configuration parsing to transformer spec test runner #4654

schlessera opened this issue May 5, 2020 · 2 comments · Fixed by #4662
Assignees
Labels
Optimizer Testing Issues related with Unit, E2E, Smoke, and other testing requirements/needs WS:Perf Work stream for Metrics, Performance and Optimizer
Milestone

Comments

@schlessera
Copy link
Collaborator

Feature description

The language agnostic spec tests in the amp-toolbox package use a leading HTML comment to configure individual tests: https://github.com/ampproject/amp-toolbox/blob/bb242b62266a71c777437d199161ba850ddab53b/packages/optimizer/spec/transformers/valid/AmpBoilerplateTransformer/always_inlines_v0css/input.html#L1-L6

This is now being used for a few tests that we are running as well, so we need to implement the same thing.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • Leading HTML comments with arguments should properly configure the transformer spec tests.

Implementation brief

  • Check if the input document starts with an HTML comment.
  • If yes, fetch the HTML comment and strip it from the input file.
  • Use json_decode() on the text content of the comment.
  • The resulting JSON document represents the options that should be passed into the transformer(s).

Note: This might require mapping from Node.js options to PHP options if these are not the same. Alternatively, we might consider adapting the options to keep them in sync with the Node.js version.

QA testing instructions

Demo

Changelog entry

@schlessera schlessera added Optimizer Testing Issues related with Unit, E2E, Smoke, and other testing requirements/needs labels May 5, 2020
@westonruter
Copy link
Member

Please prioritize this so that builds will pass again.

schlessera added a commit that referenced this issue May 7, 2020
@kmyram kmyram modified the milestones: v1.5.4, v1.6 May 27, 2020
@kmyram kmyram closed this as completed Jun 2, 2020
@westonruter westonruter modified the milestones: v1.6, v1.5.4 Jun 2, 2020
@pierlon
Copy link
Contributor

pierlon commented Jun 17, 2020

I don't think can be QA'd since it pertains to parsing of Optimizer spec tests, unless I'm mistaken @schlessera?

@schlessera schlessera added the WS:Perf Work stream for Metrics, Performance and Optimizer label Aug 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Optimizer Testing Issues related with Unit, E2E, Smoke, and other testing requirements/needs WS:Perf Work stream for Metrics, Performance and Optimizer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants