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 input file configuration parsing for spec tests #4662

Merged
merged 13 commits into from
May 7, 2020

Conversation

schlessera
Copy link
Collaborator

@schlessera schlessera commented May 6, 2020

Summary

This PR includes the following changes:

  • Updates spec files to latest amp-toolbox version.
  • Adapts SpecTest.php to extract the HTML comment with configuration arguments from the input HTML files of the spec test suite.
  • Adds a STYLES configuration key to the AmpRuntimeCss configuration.
  • Overrides stylesheet fetching with the stylesheet provided via the STYLES config argument.
  • Removes the empty <style amp-runtime> element if the stylesheet ended up being linked to.
  • Adds additional tests for AmpRuntimeCss to ensure configuration arguments behave as expected.
  • Removes temporary StubbedRequests filtering from Stub request based on test scenario #4588
  • Adds .gitattributes file to denote generated files, so that these are collapsed by default in code review.

Fixes #4654

Checklist

  • My pull request is addressing an open issue (please create one otherwise).
  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@schlessera schlessera added Optimizer Testing Issues related with Unit, E2E, Smoke, and other testing requirements/needs labels May 6, 2020
@googlebot googlebot added the cla: yes Signed the Google CLA label May 6, 2020
@@ -0,0 +1,2 @@
lib/optimizer/resources/local_fallback/* linguist-generated=true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL about using .gitattributes to suppress diffs for generated code (docs) 😄.

Would the path lib/optimiser/ be a better place for this file?

Copy link
Member

@westonruter westonruter May 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's right. Placing it into lib/optimizer will work, per Git docs:

image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as GitHub recognizes this, as their docs only mention the repo root.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assumed it would only work in the project root, but I can try moving it around to see what happens.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it doesn't work when the .gitattributes file is in a subfolder. I'll revert that change again and leave it in the root for now. This will need to be adapted once we extract the libraries into separate repositories.

Image 2020-05-07 at 8 29 08 AM

lib/optimizer/tests/SpecTest.php Outdated Show resolved Hide resolved
lib/optimizer/tests/SpecTest.php Outdated Show resolved Hide resolved
@westonruter westonruter merged commit 711911c into develop May 7, 2020
@westonruter westonruter deleted the add/4654-spec-test-configuration-parsing branch May 7, 2020 06:43
@pierlon pierlon restored the add/4654-spec-test-configuration-parsing branch May 22, 2020 15:06
@pierlon pierlon deleted the add/4654-spec-test-configuration-parsing branch May 22, 2020 15:08
pierlon added a commit that referenced this pull request May 22, 2020
* Update spec files

* Adapt spec test to extract configuration arguments from input files

* Add STYLES configuration key to AmpRuntimeCss transformer

* Nake use of styles provided via config if available

* Remove runtime style tag if stylesheet is linked

* Add more tests to assert runtime transformer behavior

* Complete stubbed requests data

* Remove unused import

* Add git attributes file to mark certain file as being generated

* Use substr() instead of a replacement for removing the leading comment

* Remove redundant JSONOBJECT_AS_ARRAY constant

* Move .gitattributes file into lib/optimizer folder

* Revert "Move .gitattributes file into lib/optimizer folder"
@pierlon pierlon added this to the v1.6 milestone Jun 2, 2020
@westonruter westonruter modified the milestones: v1.6, v1.5.4 Jun 2, 2020
@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
cla: yes Signed the Google CLA 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 this pull request may close these issues.

Add configuration parsing to transformer spec test runner
4 participants