-
Notifications
You must be signed in to change notification settings - Fork 10.3k
Update ChromeDriver version #36107
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
Update ChromeDriver version #36107
Conversation
Version compatible with Chrome 93 is required
/azp run |
Azure Pipelines successfully started running 2 pipeline(s). |
Same "91 instead of 93" message in the quarantined step. Does this step use the |
Is there a JavaScript driver that also needs to be updated to align w/ the new Chrome❔ |
Didn't find anything else related to selenium. Who would know? |
@dougbu do you think I can merge this or should all the tests pass with that change? |
Brennan mentioned this one to me:
Though the current version is 2 years old, there has been many updates in the meantime. |
I updated the minor version only of the npm package. It is 3 months old. |
I didn't find this one though. |
@sebastienros the problem here appears to be the new package.lock file. Should use |
Sorry, didn't realize the file was new, thought it was updating the whole content. |
Updated all yarn.lock files that depend on selenium package. |
<SeleniumWebDriverVersion>4.0.0-beta4</SeleniumWebDriverVersion> | ||
<SeleniumSupportVersion>4.0.0-rc1</SeleniumSupportVersion> | ||
<SeleniumWebDriverChromeDriverVersion>93.0.4577.1500</SeleniumWebDriverChromeDriverVersion> | ||
<SeleniumWebDriverVersion>4.0.0-rc1</SeleniumWebDriverVersion> |
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.
I defer to @dotnet/aspnet-blazor-eng for an actual approval |
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.
LGTM. As far as I know there shouldn't be any issues in updating src/ProjectTemplates
, if anything we can address later. Let's get this merged to get the quarantine pipeline orange again. 😄
@SteveSandersonMS @javiercn Would you have an idea about why the driver is failing, even after this change? The result is that because there is a retry logic (3 times) in the browser connection management, with increasing timeout, the pipeline fails with a global timeout and blocks PRs. A mitigation for now would be to intercept the connection issue and not retry in this case, or reduce the max delay after a few retries. c.f. logs:
|
/backport to release/6.0 |
/backport to release/5.0 |
Started backporting to release/6.0: https://github.com/dotnet/aspnetcore/actions/runs/1210151267 |
Started backporting to release/5.0: https://github.com/dotnet/aspnetcore/actions/runs/1210151501 |
@HaoK backporting to release/5.0 failed, the patch most likely resulted in conflicts: $ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: Update ChromeDriver version
Using index info to reconstruct a base tree...
M eng/Versions.props
Falling back to patching base and 3-way merge...
Auto-merging eng/Versions.props
CONFLICT (content): Merge conflict in eng/Versions.props
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Update ChromeDriver version
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128 Please backport manually! |
FYI as part of #36207 I changed the code to detect if it's running in Azure Pipelines, and if so, to use the chromedriver binary that's preinstalled on the host image, which in turn should always match the Chrome binary. So we should no longer get version mismatches in CI when Chrome is updated. However, the |
NB: I just found out there is a readme in the Components folder (https://github.com/dotnet/aspnetcore/tree/main/src/Components). Didn't think about looking for one when I wanted to run the tests, and all my attempts to run the tests failed. Something I don't see however is how to debug the tests in VS, which I was not successful in either :/ |
@SteveSandersonMS @sebastienros does this still happen after the "mergethon"? @SteveSandersonMS I don't remember the exact details of how this worked, I mainly updated the versions in eng\Versions.props and ran the tests locally to ensure they were working. |
The E2E tests work directly from VS 2022 for me without version changes, so I'm not sure what difficulties you're seeing. That is, I can right-click on a test in the Test Explorer pane and choose Debug Test, and it does run under the debugger. |
Could you clarify what is the "this" in your question? |
@SteveSandersonMS the specific details about how we picket the selenium driver and the selenium-config.json stuff, I might have missed something on the update? Everything seemed to work for me, so it's not clear to me if there's any issue we need solving? I might have misread the thread |
Last week running an E2E test in VS was throwing exceptions with selenium instance issues. Today, it can't even build the solution (./startvs.cmd from the Components folder) without crashing or hanging indefinitely (VS2022). |
@javiercn The version-picking logic I was referring to is this, so yes it is still there in
It's possible you might be missing some dependency. I was running the E2E tests from VS a lot over the last couple of weeks. If you can still repro the issue, please let me know and I'll work with you to track down what's missing or wrong.
That's new to me. My normal workflow is first building on the CLI via |
Yep, have already reinstalled dependencies, called ./build.cmd successfully. But building in VS takes ages, so does starting a test. I'll blame my laptop. VS still had the trace for the failing test before the "mergethon":
|
Switch to Build (ignore intellisense) |
I'm able to build and run tests without an issue from a clean environment |
@SteveSandersonMS have you seen the error that @sebastienros is pointing out? I've seen that in the past "sporadically" |
I've seen the "connection refused" error occur when the Selenium (Java) server was left running by a previous test run attempt, and I had to kill all the Java and Node processes in the OS task manager. I also wonder if this might happen if you actually didn't have a usable |
Worked on the command line at least with
|
Yeah, it's not something I'd run locally just for the entertainment value. But it's only 23 mins or so in CI :) https://dev.azure.com/dnceng/public/_build/results?buildId=1352586&view=logs&j=fe94c0c9-bb8c-5d6f-3b51-887173cc2f5c&t=2128ef37-3fe5-5980-876d-edb5d5b391c1 |
Fixes #36048