Skip to content

Adds playwright to test whether typescript.js runs in the browser #35471

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

Merged
merged 3 commits into from
Feb 13, 2020

Conversation

orta
Copy link
Contributor

@orta orta commented Dec 3, 2019

Makes it feasible to test for bugs like #35469

It does this by using the puppeteer to run the JS inside a headless copy of chromium.

It's not on CI, but it fails correctly:

/cc @dsherret

Screen Shot 2019-12-03 at 12 41 43 PM

@weswigham
Copy link
Member

weswigham commented Dec 3, 2019

This is only worth having if we run it on CI - we used to have this (in fact, we'd run the entire test suite in a browser frame), but removed it because it'd constantly break (in the harness) and nobody'd notice for weeks.

@orta
Copy link
Contributor Author

orta commented Dec 3, 2019

I think this might be more stable then (it's only trying to run ts) - but if this is the same kind of infra we had before I'm cool with closing.

@weswigham
Copy link
Member

I'm fine with reading it, just only if we automatically check it.

@orta
Copy link
Contributor Author

orta commented Jan 21, 2020

Bringing back this thread from idle-ness, @weswigham is running it on the GitHub Actions acceptable?

@weswigham
Copy link
Member

Hm. So we added the action definitions at the request of a GH staffer to help test them, afaik. They're actually slightly incomplete compared to our devops definitions (mostly missing environment variables that make the tests to do more things, but also lacking our extended suite of definitions for on demand tasks). Unless we decide to deprecate devops (and afaik we've not), I'd rather have it as a devops definition; just so we don't have some devops-only definitions and some actions-only definitions.

@sandersn
Copy link
Member

@orta It looks to me that this PR needs a devops pipeline definition instead of an Actions definition and it'll be ready to go. Did I get that right?

@weswigham have our plans changed re actions vs pipelines? I think I overheard a discussion about it between you and @andrewbranch.

@weswigham
Copy link
Member

Yeah, our strategy there has changed in the intervening time. We're probably going to try to make new tasks as actions rather than pipelines, wherever possible, and probably try to port over the pipelines that already exist whenever we have time, just since it seems like msft is really going for actions being the primary msft-sponsored CI for GH (vs pipelines). This is maybe complicated for things requiring secrets? I'm not sure. But for normal test tasks, like this, everything should be OK.

@orta
Copy link
Contributor Author

orta commented Feb 13, 2020

Yep, it would be complicated for things which need secrets (forks don't get them)

Perhaps more of those could be on a request basis with typescript-bot?

@weswigham
Copy link
Member

So long as secrets can be available to manually triggered actions, yeah.

@orta
Copy link
Contributor Author

orta commented Feb 13, 2020

They are ( that's how the playground PR builds get triggered )

Write-up here: https://github.com/microsoft/TypeScript-Blog-Posts/pull/9/files#diff-bb6f7746ede87d80ee3fa63e30118240R85

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Feb 13, 2020

Any thoughts on using Playwright instead so that we can get cross-browser builds?

@orta
Copy link
Contributor Author

orta commented Feb 13, 2020

Sure, it wasn't public when I made this PR

@orta
Copy link
Contributor Author

orta commented Feb 13, 2020

OK, this has been updated to use Playwright, here's what the errors would look like if the typescript.js doesnt run in the browser:

[14:01:00] Using gulpfile ~/dev/typescript/typescript-compiler/Gulpfile.js
[14:01:00] Starting 'test-browser-integration'...
[14:01:00] > /home/orta/.nvm/versions/node/v12.15.0/bin/node scripts/browserIntegrationTest.js
There was an error running built/typescript.js in chromium
Error: SyntaxError: Unexpected identifier
There was an error running built/typescript.js in firefox
Error: SyntaxError: missing ) in parenthetical
undefined
There was an error running built/typescript.js in webkit
Error: SyntaxError: Unexpected identifier 'is'. Expected ')' to end a compound expression.
[14:01:05] 'test-browser-integration' errored after 4.31 s
[14:01:05] Error: Process exited with code: 1
    at ChildProcess.<anonymous> (/home/orta/dev/typescript/typescript-compiler/scripts/build/utils.js:54:28)
    at ChildProcess.emit (events.js:223:5)
    at ChildProcess.EventEmitter.emit (domain.js:498:23)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:272:12)

@orta orta merged commit 19c3bcb into microsoft:master Feb 13, 2020
@DanielRosenwasser DanielRosenwasser changed the title Adds puppeteer to test whether typescript.js runs in the browser Adds playwright to test whether typescript.js runs in the browser Feb 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants