Closed floehopper closed 7 months ago
Chromium users can upgrade selenium-webdriver to 4.17 without this change, right?
If I'm understanding selenium-webdriver's changelog correctly, this change would make jasmine-browser-runner incompatible with Chrome 114-118. In theory that's ok, because older versions of Chrome are not supported. But neither is Chromium. So unless I'm missing something, this would make things a little easier for users of one unsupported browser at the expense of cutting off users of another unsupported browser entirely. That doesn't sound like a good trade. I appreciate the PR but I'm inclined to hold off on bumping the minimum selenium-webdriver version until there's a more compelling reason.
Chromium users can upgrade selenium-webdriver to 4.17 without this change, right?
Yes, that's correct - see https://github.com/alphagov/signon/pull/2723.
If I'm understanding selenium-webdriver's changelog correctly, this change would make jasmine-browser-runner incompatible with Chrome 114-118. In theory that's ok, because older versions of Chrome are not supported. But neither is Chromium. So unless I'm missing something, this would make things a little easier for users of one unsupported browser at the expense of cutting off users of another unsupported browser entirely. That doesn't sound like a good trade. I appreciate the PR but I'm inclined to hold off on bumping the minimum selenium-webdriver version until there's a more compelling reason.
I hadn't noticed the Chrome compatibility changes. What you say sounds reasonable. The only thing I'd add that the problem with Chromium manifested in a very non-obvious error when running in a Docker container on MacOS with Apple Silicon.
However, if I mention the error in a subsequent comment on this PR, perhaps that'll be sufficient for others with the same problem to find the solution.
The error I was seeing when running Jasmine specs in a Docker container on MacOS with Apple Silicon was:
SessionNotCreatedError: session not created: Chrome failed to start: exited normally.
See this PR for a full diagnosis.
To include this fix for correct detection of a chromium binary.