-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Conversation
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. |
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. |
I'm fine with reading it, just only if we automatically check it. |
Bringing back this thread from idle-ness, @weswigham is running it on the GitHub Actions acceptable? |
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. |
@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. |
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. |
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? |
So long as secrets can be available to manually triggered actions, yeah. |
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 |
Any thoughts on using Playwright instead so that we can get cross-browser builds? |
Sure, it wasn't public when I made this PR |
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:
|
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