Skip to content

🏗 Build and test AMP on Windows#29115

Merged
rsimha merged 8 commits intoampproject:masterfrom
rsimha:2020-07-01-WindowsTesting
Aug 20, 2020
Merged

🏗 Build and test AMP on Windows#29115
rsimha merged 8 commits intoampproject:masterfrom
rsimha:2020-07-01-WindowsTesting

Conversation

@rsimha
Copy link
Copy Markdown
Contributor

@rsimha rsimha commented Jul 1, 2020

PR Highlights:

  • Fixes AMP build on Windows by normalizing paths that used to have hard-coded separators
  • Runs gulp dist and builds minified AMP on Windows on GH actions (blocking check)
  • Runs all unit and integration tests on Edge (blocking check)
  • Runs integration tests that have opted in to IE 11 (some failures, so not blocking for now)

Screenshots:

gulp dist on Windows:

image
...
image

gulp unit on Edge:

image

gulp integration on Edge:

image

gulp integration on IE 11:

image

Future work: Fix tests on IE, make build-system/ more platform independent

Unblocked by #28208
Follow up to #29012
Was blocked by #27570 (comment)
Unblocked by ampproject/amp-closure-compiler#23 plus other commits
Unblocked by #29903
Addresses #27520 (comment)
Fixes #27570

@rsimha rsimha self-assigned this Jul 1, 2020
@rsimha
Copy link
Copy Markdown
Contributor Author

rsimha commented Jul 9, 2020

Blocked by #27570 (comment)

@rsimha
Copy link
Copy Markdown
Contributor Author

rsimha commented Aug 20, 2020

Unblocked by ampproject/amp-closure-compiler#23 and #29903

@rsimha rsimha requested a review from kristoferbaxter August 20, 2020 01:03
@rsimha rsimha marked this pull request as ready for review August 20, 2020 01:03
@rsimha rsimha requested a review from samouri August 20, 2020 01:04
@rsimha
Copy link
Copy Markdown
Contributor Author

rsimha commented Aug 20, 2020

This PR has sat around for more than a month while I've battled with various parts of the compilation toolchain. At long last, we have a fully working minified build on Windows, and fully working unit and integration tests on Edge.

I'm sending this out for review so we can merge a blocking check for compilation on Windows and tests on Edge, and deal with IE test failures via a separate PR. Meanwhile, the current status on IE can be found in the PR description. (/cc @choumx)

/cc @samouri who is looking to use this on a windows workstation. (Please check out this branch and let me know how it works for you.)

@rsimha rsimha requested a review from dreamofabear August 20, 2020 01:19
@rsimha rsimha changed the title 🏗 Build and test on Windows on GH Actions 🏗 Build and test AMP on Windows Aug 20, 2020
Copy link
Copy Markdown
Contributor

@kristoferbaxter kristoferbaxter left a comment

Choose a reason for hiding this comment

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

Looks good to me, but I am not on a Windows machine so would defer to someone who can take a look under Windows.

@rsimha
Copy link
Copy Markdown
Contributor Author

rsimha commented Aug 20, 2020

I am not on a Windows machine so would defer to someone who can take a look under Windows.

I've tested this PR on a Windows VM that I installed on my personal Mac computer. Based on that, and on multiple green Windows runs on GH Actions, I'm going to merge this. Happy to address other Windows inconsistencies / bugs in follow up PRs.

@rsimha rsimha merged commit 4f86770 into ampproject:master Aug 20, 2020
@rsimha rsimha deleted the 2020-07-01-WindowsTesting branch August 20, 2020 17:05
@jaygray0919
Copy link
Copy Markdown

We ran into gulp issues when trying to implement https://github.com/ampproject/amp.dev on WSL (Win 10). Will try again next week. Our fallback is to use a Mac (a supported OS); but WSL is preferred.

@samouri
Copy link
Copy Markdown
Member

samouri commented Sep 2, 2020

@jaygray0919: can you open up an issue on the amp.dev repo? This issue is only about the amphtml one.

FWIW, here are some items I've noticed about windows dev in the past week:

  1. The babel unit tests break on windows
  2. Things break when trying to use both WSL and regular Windows within the same folder, I have two clones of the repo to avoid weird compat issues.
  3. The Karma tests don't work within WSL (at least not without manually setting CHROME_BIN env var, but still I ran in to issues), but work perfectly when I run them in regular windows
  4. Windows Terminal is pretty nice :)

ed-bird pushed a commit to ed-bird/amphtml that referenced this pull request Dec 10, 2020
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.

Add Github Action Windows Development Sanity Check

7 participants