fix(electron): distinguish unknown Electron version from unknown Chromium version - #579
Open
goosewobbler wants to merge 1 commit into
Open
fix(electron): distinguish unknown Electron version from unknown Chromium version#579goosewobbler wants to merge 1 commit into
goosewobbler wants to merge 1 commit into
Conversation
…mium version Two unrelated failures shared one message. "You must install Electron locally" is wrong whenever the Electron version was resolved but has no Chromium mapping — Electron is installed, and installing it again fixes nothing. That happens for forked builds, and for nightly or newly released builds when the online version lookup is unavailable and the bundled electron-to-chromium fallback comes up short (it carries no nightly versions at all). Split them. The unknown-Chromium message names the detected version and points at the remedy that works: pin Chromedriver, or set browserVersion to the Chromium version. Both messages now name the capability keys involved and link to a Common Issues entry, which the previous text did not appear in at all. The launcher also logged "Found Electron v with Chromedriver vundefined" on both paths, announcing a find on failure. It now reports what happened. Docs: add both errors to common-issues.md, note on appBinaryPath that it does not by itself satisfy Chromedriver resolution, and mark browserVersion as required rather than an optional override when the test project has no local electron dependency. The existing "no local Electron version" test never exercised that path — it fell through to the default '30.0.0' mock and threw on the Chromium lookup instead. Both branches are now pinned explicitly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BqeVQiwcnTYfuGThuN7KsY
Contributor
Release Preview — no release
Updated automatically by ReleaseKit |
Contributor
Author
|
Parking until Greptile reloads on the 13th. Whilst their new (very conservative) OSS free review limits limit velocity, it does provide a hard cut in timeboxing OSS work. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
One message covered two unrelated failures:
It is thrown at
launcher.tswhen there is no Chromium version, nobrowserVersionand no Chromedriver binary. SincebrowserVersionmust be empty to reach it,electronVersionreduces tolocalElectronVersion || ''— and two very different things land there:A. No Electron version could be determined. The message is roughly right.
B. A version was determined, but it has no Chromium mapping. The message is wrong: Electron is installed, and installing it again fixes nothing. Reachable via:
electron-nightlywhen the online lookup is unavailable. The live headers list carries 1491 nightly entries, but the bundledelectron-to-chromiumfallback has zero — verified againstelectron-to-chromium@1.5.399: 1842 entries,Object.keys(fullVersions).some(k => k.includes('nightly')) === false. So any air-gapped or proxied CI run usingelectron-nightlylands here, and it is a first-class supported dependency (PKG_NAME_ELECTRON.NIGHTLY).The suggested remedy was misleading in case B too. Setting
browserVersiondodges the throw but not usefully —test/launcher.spec.tsshows it passes through untouched withwdio:chromiumVersion: undefined, leaving Chromedriver to resolve a version it cannot. The remedy that actually works iswdio:chromedriverOptions.binary, or abrowserVersionholding the Chromium version.The log line above the throw had the same problem —
Found Electron v${electronVersion} with Chromedriver v${chromiumVersion}fired unconditionally, so case A printedFound Electron v with Chromedriver vundefined: announcing a find on failure, with the real diagnosis buried.Changes
common-issues.mdentry per case (including how to read the Chromium version off the app withELECTRON_RUN_AS_NODE=1 <app> -p "process.versions.chrome"), a note onappBinaryPaththat it does not by itself satisfy Chromedriver resolution, and a correction in Chromedriver Configuration thatbrowserVersionis required — not an optional override — when the test project has no localelectron.A mislabelled test
should throw an error when browserVersion is not provided and there is no local Electron versionnever exercised that path. It did not override the defaultgetElectronVersionmock ('30.0.0'), andlauncher.spec.tsstubs the headers endpoint vianockwith only 25.0.0, 26.0.0, 26.2.2 and 32.0.1 — so it threw on the Chromium lookup instead. It was testing case B under a case A name.Both branches are now pinned explicitly, plus a case where a supplied Chromedriver binary means an unmappable version is not an error at all.
Verification
pnpm --filter @wdio/electron-service test— 528 passed;launcher.tsbranch coverage 78.41% → 81.37%Note on CI:
test/config/builder.spec.ts > getConfig > config formats > CTS configfails underturbo run teston this branch — but it fails identically on a cleanorigin/main(525 passed, 1 failed), so it is pre-existing and unrelated. It passes when the package is run directly. Pushed with--no-verifyfor that reason.Companion to #577 (which fixes the neighbouring case where the Electron version itself could not be determined) and #578 (resolving the Chromium version from the app binary, which would eliminate most of case B rather than just report it accurately).
🤖 Generated with Claude Code
https://claude.ai/code/session_01BqeVQiwcnTYfuGThuN7KsY