-
Notifications
You must be signed in to change notification settings - Fork 10.3k
Try caching playwright browsers download #33557
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
/azp run |
Azure Pipelines successfully started running 2 pipeline(s). |
caf55eb
to
cc5a930
Compare
/azp-run |
/azp run |
Azure Pipelines successfully started running 2 pipeline(s). |
@dotnet/aspnet-build does anyone know why only this PR seems to fail with this error? I added some azure magic to try and cache our playwright browsers, but it shouldn't affect the mac os test leg at all... I've tried like 4-5+ times now Error: Failed find: ENOENT: no such file or directory, lstat '/System/Library/Frameworks/WebKit.framework/Versions/A/Frameworks/WebKitLegacy.framework/Versions/Versions' |
Very weird, I'm not sure. Could be an AzDO issue w/ the |
I haven't seen any ongoing issues with the @dotnet/dnceng for visibility. |
If you scroll up to the "run build.sh" stage there's lots of restore failures (new variant, same problem) in that stage. It's possible this incomplete restore has some impact?
|
.azure/pipelines/ci.yml
Outdated
inputs: | ||
key: 'pw | "$(Agent.OS)" | eng/versions.props' | ||
restoreKeys: | | ||
pw | "$(Agent.OS)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why fallback to whatever Playwright version has been cached❔ Suggest leaving restoreKeys
out
cacheHitVar: CACHE_PLAYWRIGHT_HIT | ||
displayName: Cache playwright browsers | ||
- script: npm i -D playwright | ||
condition: ne(variables['CACHE_PLAYWRIGHT_HIT'], 'true') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does the Helix test job need Playwright❔ Suggest leaving this job alone
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well the quarantined version of helix will need it, and eventually the real one too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we do Playwright installation on the Helix agents❔ I could understand this if the installations were uploaded from the build agents.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now it does it both on the helix machines but also as part of local builds/tests since the developer workflow is a thing too, generally it doesn't look like our builds are factored in a way that makes it easy to optionally install dependencies. I had to do some magic with conditional compilation for that to work on helix, so its possible but currently we aren't setup to easily stop installing playwright on builds
@@ -659,6 +661,17 @@ stages: | |||
beforeBuild: | |||
- powershell: "& ./src/Servers/IIS/tools/UpdateIISExpressCertificate.ps1; & ./src/Servers/IIS/tools/update_schema.ps1" | |||
displayName: Setup IISExpress test certificates and schema | |||
- task: Cache@2 | |||
inputs: | |||
key: 'pw | "$(Agent.OS)" | eng/versions.props' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eng/Versions.props doesn't list the npm
package version for Playwright. I'm not sure where the version is stored. @javiercn❔ If we just use the latest, caching would be a Bad Thing:tm:.
9b290f4
to
b3fa42b
Compare
Co-authored-by: Doug Bunting <[email protected]>
@MattGal I'm still seeing an error with restore due to this 'https://pkgs.dev.azure.com/dnceng/9ee6d478-d288-47f7-aacc-f6e6d082ae6d/_packaging/7d9f5c21-0d79-403f-bfe3-9a4506529760/nuget/v3/flat2/netstandard.library/index.json' not existing, is something bad cached in this PR maybe? |
I've been OOF, taking a look. |
@HaoK I'm happy to look at whatever but I need a link, the only runs I can quickly find are failing just for |
Yeah that's my question, I constantly get those errors only, and its just specific to this PR where you said before it was a "(new variant, same problem)" |
/azp run |
Azure Pipelines successfully started running 2 pipeline(s). |
No description provided.