Skip to content

babel tests: support windows#30165

Merged
samouri merged 4 commits intoampproject:masterfrom
samouri:babel-windows-fixes
Sep 10, 2020
Merged

babel tests: support windows#30165
samouri merged 4 commits intoampproject:masterfrom
samouri:babel-windows-fixes

Conversation

@samouri
Copy link
Copy Markdown
Member

@samouri samouri commented Sep 9, 2020

summary
Followup to #29115.

Make the babel tests pass on Windows. There were two main items I found that needed updating:

  1. String checks against exact path strings that were posix style
  2. Using the path module functions in for strings meant to be the the "from" in ImportDeclarations. JS modules need to be posix-style.
  3. Direct checks that the root folder is amphtml. Technically the git repo can be a folder with any name.

@samouri samouri self-assigned this Sep 9, 2020
@google-cla google-cla bot added the cla: yes label Sep 9, 2020
@amp-owners-bot
Copy link
Copy Markdown

amp-owners-bot bot commented Sep 9, 2020

Hey @erwinmombay, @jridgewell! These files were changed:

build-system/babel-plugins/babel-plugin-amp-mode-transformer/index.js
build-system/babel-plugins/babel-plugin-transform-dev-methods/index.js
build-system/babel-plugins/babel-plugin-transform-inline-configure-component/index.js
build-system/babel-plugins/babel-plugin-transform-jss/create-hash.js
build-system/babel-plugins/babel-plugin-transform-promise-resolve/index.js

@samouri samouri requested a review from rsimha September 9, 2020 16:25
Copy link
Copy Markdown
Contributor

@rsimha rsimha left a comment

Choose a reason for hiding this comment

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

Thanks for fixing these. For maintainability, I've suggested some changes that may or may not work. WDYT?

@samouri
Copy link
Copy Markdown
Member Author

samouri commented Sep 9, 2020

Leaving this in a half-broken state right now. Will ping back tomorrow when its ready for another round

@samouri
Copy link
Copy Markdown
Member Author

samouri commented Sep 10, 2020

@rsimha / @jridgewell: updated, PTAL

Copy link
Copy Markdown
Contributor

@rsimha rsimha left a comment

Choose a reason for hiding this comment

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

Thanks for the fix. Glad we had these tests to detect problems on Windows.

@samouri samouri merged commit cf751b2 into ampproject:master Sep 10, 2020
@samouri samouri deleted the babel-windows-fixes branch September 14, 2020 14:41
ed-bird pushed a commit to ed-bird/amphtml that referenced this pull request Dec 10, 2020
* babel tests: support windows

* nocked a few via posix

* simplify fn

* lint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants