From d77bb74db2fa2021c26630f651830f0dfdf82c64 Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Wed, 18 Mar 2026 16:24:12 -0400 Subject: [PATCH 01/26] Enforce repository branch name rules in Rename Branch dialog Add repo rules validation to the Rename Branch dialog, matching the existing Create Branch dialog behavior. When a user types a new branch name, it is validated against the repository's branch name pattern rulesets via the API. - Added accounts and cachedRepoRulesets props to RenameBranch component - Added debounced checkBranchRules method (500ms) that calls the GitHub API to fetch and validate branch rules - Shows InputError when the user cannot bypass a failing rule (blocks rename) - Shows InputWarning when the user can bypass (allows rename with caution) - Connected aria-describedby for screen reader accessibility - Passed new props from app.tsx popup rendering --- app/src/ui/app.tsx | 2 + .../ui/rename-branch/rename-branch-dialog.tsx | 184 +++++++++++++++++- 2 files changed, 182 insertions(+), 4 deletions(-) diff --git a/app/src/ui/app.tsx b/app/src/ui/app.tsx index 686a63da130..6353c8ea707 100644 --- a/app/src/ui/app.tsx +++ b/app/src/ui/app.tsx @@ -1480,6 +1480,8 @@ export class App extends React.Component { dispatcher={this.props.dispatcher} repository={popup.repository} branch={popup.branch} + accounts={this.state.accounts} + cachedRepoRulesets={this.state.cachedRepoRulesets} onDismissed={onPopupDismissedFn} /> ) diff --git a/app/src/ui/rename-branch/rename-branch-dialog.tsx b/app/src/ui/rename-branch/rename-branch-dialog.tsx index 8738b448ee6..9a23653b888 100644 --- a/app/src/ui/rename-branch/rename-branch-dialog.tsx +++ b/app/src/ui/rename-branch/rename-branch-dialog.tsx @@ -1,35 +1,57 @@ import * as React from 'react' import { Dispatcher } from '../dispatcher' -import { Repository } from '../../models/repository' +import { + Repository, + isRepositoryWithGitHubRepository, +} from '../../models/repository' import { Branch } from '../../models/branch' import { Dialog, DialogContent, DialogFooter } from '../dialog' import { renderBranchHasRemoteWarning } from '../lib/branch-name-warnings' import { OkCancelButtonGroup } from '../dialog/ok-cancel-button-group' import { RefNameTextBox } from '../lib/ref-name-text-box' +import { API, APIRepoRuleType, IAPIRepoRuleset } from '../../lib/api' +import { Account } from '../../models/account' +import { getAccountForRepository } from '../../lib/get-account-for-repository' +import { InputError } from '../lib/input-description/input-error' +import { InputWarning } from '../lib/input-description/input-warning' +import { parseRepoRules, useRepoRulesLogic } from '../../lib/helpers/repo-rules' +import { Row } from '../lib/row' interface IRenameBranchProps { readonly dispatcher: Dispatcher readonly onDismissed: () => void readonly repository: Repository readonly branch: Branch + readonly accounts: ReadonlyArray + readonly cachedRepoRulesets: ReadonlyMap } interface IRenameBranchState { readonly newName: string + readonly currentError: { error: Error; isWarning: boolean } | null } export class RenameBranch extends React.Component< IRenameBranchProps, IRenameBranchState > { + private branchRulesDebounceId: number | null = null + + private readonly ERRORS_ID = 'rename-branch-name-errors' + public constructor(props: IRenameBranchProps) { super(props) - this.state = { newName: props.branch.name } + this.state = { newName: props.branch.name, currentError: null } } public render() { + const disabled = + this.state.newName.length === 0 || + (!!this.state.currentError && !this.state.currentError.isWarning) + const hasError = !!this.state.currentError + return ( + + {this.renderBranchNameErrors()} ) } + private renderBranchNameErrors() { + const { currentError } = this.state + if (!currentError) { + return null + } + + if (currentError.isWarning) { + return ( + + + {currentError.error.message} + + + ) + } else { + return ( + + + {currentError.error.message} + + + ) + } + } + private onNameChange = (name: string) => { - this.setState({ newName: name }) + this.setState({ newName: name, currentError: null }) + + if (this.branchRulesDebounceId !== null) { + window.clearTimeout(this.branchRulesDebounceId) + } + + if (name !== '') { + this.branchRulesDebounceId = window.setTimeout( + this.checkBranchRules, + 500, + name + ) + } + } + + /** + * Checks repo rules to see if the provided branch name is valid for the + * current user and repository. The "get all rules for a branch" endpoint + * is called first, and if a "creation" or "branch name" rule is found, + * then those rulesets are checked to see if the current user can bypass + * them. + */ + private checkBranchRules = async (branchName: string) => { + if ( + this.state.newName !== branchName || + this.props.accounts.length === 0 || + !isRepositoryWithGitHubRepository(this.props.repository) || + branchName === '' || + this.state.currentError !== null + ) { + return + } + + const account = getAccountForRepository( + this.props.accounts, + this.props.repository + ) + + if ( + account === null || + !useRepoRulesLogic(account, this.props.repository) + ) { + return + } + + const api = API.fromAccount(account) + const branchRules = await api.fetchRepoRulesForBranch( + this.props.repository.gitHubRepository.owner.login, + this.props.repository.gitHubRepository.name, + branchName + ) + + // Make sure user branch name hasn't changed during api call + if (this.state.newName !== branchName) { + return + } + + // filter the rules to only the relevant ones and get their IDs. use a Set to dedupe. + const toCheck = new Set( + branchRules + .filter( + r => + r.type === APIRepoRuleType.Creation || + r.type === APIRepoRuleType.BranchNamePattern + ) + .map(r => r.ruleset_id) + ) + + // there are no relevant rules for this branch name, so return + if (toCheck.size === 0) { + return + } + + // check for actual failures + const { branchNamePatterns, creationRestricted } = await parseRepoRules( + branchRules, + this.props.cachedRepoRulesets, + this.props.repository + ) + + // Make sure user branch name hasn't changed during parsing of repo rules + // (async due to a config retrieval of users with commit signing repo rules) + if (this.state.newName !== branchName) { + return + } + + const { status } = branchNamePatterns.getFailedRules(branchName) + + // Only possible kind of failures is branch name pattern failures and creation restriction + if (creationRestricted !== true && status === 'pass') { + return + } + + // check cached rulesets to see which ones the user can bypass + let cannotBypass = false + for (const id of toCheck) { + const rs = this.props.cachedRepoRulesets.get(id) + + if (rs?.current_user_can_bypass !== 'always') { + // the user cannot bypass, so stop checking + cannotBypass = true + break + } + } + + if (cannotBypass) { + this.setState({ + currentError: { + error: new Error( + `Branch name '${branchName}' is restricted by repo rules.` + ), + isWarning: false, + }, + }) + } else { + this.setState({ + currentError: { + error: new Error( + `Branch name '${branchName}' is restricted by repo rules, but you can bypass them. Proceed with caution!` + ), + isWarning: true, + }, + }) + } } private renameBranch = () => { From 5f1829d27e540b2de9b253d53961e6fd3eeb5b98 Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Wed, 18 Mar 2026 16:27:11 -0400 Subject: [PATCH 02/26] Add componentWillUnmount to clear debounce timer Prevents checkBranchRules from firing after the dialog is dismissed, matching the Create Branch dialog's cleanup pattern. --- app/src/ui/rename-branch/rename-branch-dialog.tsx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/src/ui/rename-branch/rename-branch-dialog.tsx b/app/src/ui/rename-branch/rename-branch-dialog.tsx index 9a23653b888..df2c4599839 100644 --- a/app/src/ui/rename-branch/rename-branch-dialog.tsx +++ b/app/src/ui/rename-branch/rename-branch-dialog.tsx @@ -46,6 +46,12 @@ export class RenameBranch extends React.Component< this.state = { newName: props.branch.name, currentError: null } } + public componentWillUnmount() { + if (this.branchRulesDebounceId !== null) { + window.clearTimeout(this.branchRulesDebounceId) + } + } + public render() { const disabled = this.state.newName.length === 0 || From 0ec4a05e7d670713f411843b43644209ff153099 Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Wed, 18 Mar 2026 16:49:47 -0400 Subject: [PATCH 03/26] Fix rename branch dialog width to prevent jarring resize Set a fixed width of 400px on the rename-branch dialog to match the create-branch dialog. Without this, the dialog expands from min-width to accommodate error/warning text when repo rules validation fires. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- app/styles/ui/_dialog.scss | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/styles/ui/_dialog.scss b/app/styles/ui/_dialog.scss index cc931bd817d..de161af4154 100644 --- a/app/styles/ui/_dialog.scss +++ b/app/styles/ui/_dialog.scss @@ -548,6 +548,9 @@ dialog { display: inline; } } + &#rename-branch { + width: 400px; + } &#push-branch-commits { width: 450px; } From cf3e05a67dba5fa31cb8905f92b7d0f4dc10342d Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Wed, 18 Mar 2026 16:55:05 -0400 Subject: [PATCH 04/26] Validate branch name against repo rules on dialog open Call checkBranchRules in componentDidMount so that when the rename branch dialog opens with a pre-filled name that violates repo rules, the error/warning is shown immediately rather than only after typing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- app/src/ui/rename-branch/rename-branch-dialog.tsx | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/src/ui/rename-branch/rename-branch-dialog.tsx b/app/src/ui/rename-branch/rename-branch-dialog.tsx index df2c4599839..66bc32907ae 100644 --- a/app/src/ui/rename-branch/rename-branch-dialog.tsx +++ b/app/src/ui/rename-branch/rename-branch-dialog.tsx @@ -46,6 +46,14 @@ export class RenameBranch extends React.Component< this.state = { newName: props.branch.name, currentError: null } } + public componentDidMount() { + // Validate the pre-filled branch name on dialog open so existing + // rule violations are shown immediately. + if (this.state.newName !== '') { + this.checkBranchRules(this.state.newName) + } + } + public componentWillUnmount() { if (this.branchRulesDebounceId !== null) { window.clearTimeout(this.branchRulesDebounceId) From e5252d9bf9691db3a7bc1332e678963da4ac64b8 Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Thu, 19 Mar 2026 13:10:38 +0100 Subject: [PATCH 05/26] Add E2E tests for critical paths --- .github/workflows/ci.yml | 131 +++++++++ app/app-info.ts | 2 +- app/test/e2e/app-launch.e2e.ts | 439 +++++++++++++++++++++++++++++ app/test/e2e/e2e-fixtures.ts | 218 ++++++++++++++ app/test/e2e/mock-update-server.ts | 218 ++++++++++++++ app/test/e2e/playwright.config.ts | 22 ++ app/test/e2e/test-helpers.ts | 64 +++++ app/test/globals.mts | 55 +++- package.json | 5 + script/playwright-electron.ts | 110 ++++++++ script/post-install.ts | 39 +++ 11 files changed, 1301 insertions(+), 2 deletions(-) create mode 100644 app/test/e2e/app-launch.e2e.ts create mode 100644 app/test/e2e/e2e-fixtures.ts create mode 100644 app/test/e2e/mock-update-server.ts create mode 100644 app/test/e2e/playwright.config.ts create mode 100644 app/test/e2e/test-helpers.ts create mode 100644 script/playwright-electron.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6aad84e2881..c590555843e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -167,3 +167,134 @@ jobs: dist/GitHubDesktopSetup-${{matrix.arch}}.msi dist/bundle-size.json if-no-files-found: error + e2e-smoke: + name: E2E Smoke ${{ matrix.friendlyName }} ${{ matrix.arch }} + runs-on: ${{ matrix.os }} + permissions: + contents: read + strategy: + fail-fast: false + matrix: + include: + - os: macos-14-xlarge + friendlyName: macOS + arch: arm64 + - os: windows-2022 + friendlyName: Windows + arch: x64 + timeout-minutes: 60 + environment: ${{ inputs.environment }} + env: + RELEASE_CHANNEL: ${{ inputs.environment }} + steps: + - uses: actions/checkout@v4 + with: + repository: ${{ inputs.repository || github.repository }} + ref: ${{ inputs.ref }} + submodules: recursive + - uses: actions/setup-python@v5 + with: + python-version: '3.11' + - name: Use Node.js ${{ env.NODE_VERSION }} + uses: actions/setup-node@v4 + with: + node-version: ${{ env.NODE_VERSION }} + cache: yarn + - name: Install ffmpeg + if: runner.os == 'Windows' + run: choco install ffmpeg --yes --no-progress + - name: Install and build dependencies + run: yarn + env: + npm_config_arch: ${{ matrix.arch }} + TARGET_ARCH: ${{ matrix.arch }} + - name: Build production app + run: yarn build:prod + env: + DESKTOP_E2E_UPDATES_URL: http://127.0.0.1:51789/update + DESKTOP_OAUTH_CLIENT_ID: ${{ secrets.DESKTOP_OAUTH_CLIENT_ID }} + DESKTOP_OAUTH_CLIENT_SECRET: + ${{ secrets.DESKTOP_OAUTH_CLIENT_SECRET }} + APPLE_ID: ${{ secrets.APPLE_ID }} + APPLE_ID_PASSWORD: ${{ secrets.APPLE_ID_PASSWORD }} + APPLE_TEAM_ID: ${{ secrets.APPLE_TEAM_ID }} + APPLE_APPLICATION_CERT: ${{ secrets.APPLE_APPLICATION_CERT }} + KEY_PASSWORD: ${{ secrets.APPLE_APPLICATION_CERT_PASSWORD }} + npm_config_arch: ${{ matrix.arch }} + TARGET_ARCH: ${{ matrix.arch }} + - name: Install Azure Code Signing Client + if: ${{ runner.os == 'Windows' && inputs.sign }} + run: | + $acsZip = Join-Path $env:RUNNER_TEMP "acs.zip" + $acsDir = Join-Path $env:RUNNER_TEMP "acs" + Invoke-WebRequest -Uri https://www.nuget.org/api/v2/package/Microsoft.Trusted.Signing.Client/1.0.95 -OutFile $acsZip -Verbose + Expand-Archive $acsZip -Destination $acsDir -Force -Verbose + Copy-Item -Path "C:\Program Files (x86)\Windows Kits\10\bin\10.0.22621.0\x64\*" -Include signtool.exe,signtool.exe.manifest,Microsoft.Windows.Build.Signing.mssign32.dll.manifest,mssign32.dll,Microsoft.Windows.Build.Signing.wintrust.dll.manifest,wintrust.dll,Microsoft.Windows.Build.Appx.AppxSip.dll.manifest,AppxSip.dll,Microsoft.Windows.Build.Appx.AppxPackaging.dll.manifest,AppxPackaging.dll,Microsoft.Windows.Build.Appx.OpcServices.dll.manifest,OpcServices.dll -Destination "node_modules\electron-winstaller\vendor" -Verbose + - name: Azure Login (OIDC) + if: ${{ runner.os == 'Windows' && inputs.sign }} + uses: azure/login@v2 + with: + client-id: ${{ secrets.AZURE_CODE_SIGNING_CLIENT_ID }} + tenant-id: ${{ secrets.AZURE_CODE_SIGNING_TENANT_ID }} + allow-no-subscriptions: true + - name: Package production app + run: yarn package + env: + npm_config_arch: ${{ matrix.arch }} + AZURE_TENANT_ID: ${{ secrets.AZURE_CODE_SIGNING_TENANT_ID }} + AZURE_CLIENT_ID: ${{ secrets.AZURE_CODE_SIGNING_CLIENT_ID }} + - name: Install app on macOS + if: runner.os == 'macOS' + run: | + rm -rf "/Applications/GitHub Desktop.app" + ditto "dist/GitHub Desktop-darwin-arm64/GitHub Desktop.app" "/Applications/GitHub Desktop.app" + echo "DESKTOP_E2E_APP_PATH=/Applications/GitHub Desktop.app/Contents/MacOS/GitHub Desktop" >> "$GITHUB_ENV" + - name: Install app on Windows + if: runner.os == 'Windows' + shell: pwsh + run: | + function Write-SquirrelLogs { + $logPaths = @( + "$env:LOCALAPPDATA\SquirrelSetup.log", + "$env:LOCALAPPDATA\GitHubDesktop\SquirrelSetup.log" + ) + + foreach ($logPath in $logPaths) { + if (Test-Path $logPath) { + Write-Host "Showing log: $logPath" + Get-Content $logPath -Tail 200 + } + } + } + + $setupExe = "dist/GitHubDesktopSetup-${{ matrix.arch }}.exe" + $installer = Start-Process -FilePath $setupExe -ArgumentList "/S" -PassThru + + try { + Wait-Process -Id $installer.Id -Timeout 300 -ErrorAction Stop + } catch { + Write-SquirrelLogs + throw "Windows installer timed out after 300 seconds" + } + + Get-Process GitHubDesktop -ErrorAction SilentlyContinue | Stop-Process -Force + + $installedExe = $null + for ($attempt = 0; $attempt -lt 30 -and -not $installedExe; $attempt++) { + $installedExe = Get-ChildItem "$env:LOCALAPPDATA\GitHubDesktop\app-*\GitHubDesktop.exe" -ErrorAction SilentlyContinue | + Sort-Object FullName -Descending | + Select-Object -First 1 -ExpandProperty FullName + + if (-not $installedExe) { + Start-Sleep -Seconds 2 + } + } + + if (-not $installedExe) { + Write-SquirrelLogs + throw "Unable to locate installed GitHub Desktop executable" + } + + Add-Content -Path $env:GITHUB_ENV -Value "DESKTOP_E2E_APP_PATH=$installedExe" + - name: Run E2E smoke tests + run: yarn test:e2e:run diff --git a/app/app-info.ts b/app/app-info.ts index e29b1a09b32..ec49654d493 100644 --- a/app/app-info.ts +++ b/app/app-info.ts @@ -25,7 +25,7 @@ export function getReplacements() { __DEV__: isDevBuild, __DEV_SECRETS__: isDevBuild || !process.env.DESKTOP_OAUTH_CLIENT_SECRET, __RELEASE_CHANNEL__: s(channel), - __UPDATES_URL__: s(getUpdatesURL()), + __UPDATES_URL__: s(process.env.DESKTOP_E2E_UPDATES_URL ?? getUpdatesURL()), __SHA__: s(getSHA()), 'process.platform': s(process.platform), 'process.env.NODE_ENV': s(process.env.NODE_ENV || 'development'), diff --git a/app/test/e2e/app-launch.e2e.ts b/app/test/e2e/app-launch.e2e.ts new file mode 100644 index 00000000000..b264606a011 --- /dev/null +++ b/app/test/e2e/app-launch.e2e.ts @@ -0,0 +1,439 @@ +/** + * E2E tests for GitHub Desktop using Playwright + Electron. + * + * These tests launch the real production-built app, interact with it + * via Playwright, and verify core functionality end-to-end. Video and + * trace recording are enabled via playwright.config.ts. + */ + +import { + test, + expect, + controlMockServer, + getMockRequests, + dismissMoveToApplicationsDialog, + terminateWindowsUpdaterProcesses, +} from './e2e-fixtures' +import { + smokeRepoFileContents, + smokeRepoFileName, + smokeRepoPath, + getSmokeRepoCurrentBranch, + getSmokeRepoHeadMessage, + getSmokeRepoStatus, +} from './test-helpers' +import type { Page } from '@playwright/test' + +// All tests run sequentially in the same Electron session. +test.describe.configure({ mode: 'serial' }) + +async function failIfAppErrorDialogIsVisible(page: Page) { + const appErrorDialog = page.locator('dialog#app-error') + const isVisible = await appErrorDialog.isVisible().catch(() => false) + + if (!isVisible) { + return + } + + const title = + (await appErrorDialog.locator('.dialog-header h1').textContent()) ?? '' + const description = + (await page + .locator('#app-error-description') + .textContent() + .catch(() => null)) ?? '' + + throw new Error( + `App error dialog blocked the E2E flow. Title: ${title.trim()} Description: ${description.trim()}` + ) +} + +function isMockUpdateRequest(url: string) { + return ( + url.includes('/update') || + url.includes('/RELEASES') || + url.endsWith('.nupkg') || + url.startsWith('/download/') + ) +} + +// ── Smoke tests ───────────────────────────────────────────────────── + +test.describe('GitHub Desktop - App Launch', () => { + test('should launch, complete welcome flow, commit, and switch branches', async ({ + mainWindow: page, + }) => { + // Wait for the React app to mount + await page.waitForFunction( + () => + (document.getElementById('desktop-app-container')?.innerHTML.length ?? + 0) > 100, + null, + { timeout: 30000 } + ) + + // ── Welcome flow ──────────────────────────────────────────────── + const skipButton = page.locator('a.skip-button') + await skipButton.waitFor({ state: 'visible', timeout: 30000 }) + await skipButton.click() + + const nameInput = page.locator('input[placeholder="Your Name"]') + await nameInput.waitFor({ state: 'visible', timeout: 15000 }) + if ((await nameInput.inputValue()) === '') { + await nameInput.fill('GitHub Desktop E2E') + } + + const emailInput = page.locator( + 'input[placeholder="your-email@example.com"]' + ) + if ((await emailInput.inputValue()) === '') { + await emailInput.fill('desktop-e2e@example.com') + } + + await page.locator('button:has-text("Finish")').click() + await page.waitForSelector('#welcome', { state: 'hidden', timeout: 15000 }) + + await dismissMoveToApplicationsDialog(page) + + // ── Repository view ───────────────────────────────────────────── + const repoFile = page + .locator(`//*[contains(normalize-space(), "${smokeRepoFileName}")]`) + .first() + const addButton = page + .locator( + '//*[contains(normalize-space(), "Add an Existing Repository from your Local Drive") or contains(normalize-space(), "Add an Existing Repository from your local drive")]' + ) + .first() + const addRepositoryDialog = page.locator('dialog#add-existing-repository') + + await Promise.race([ + repoFile.waitFor({ state: 'visible', timeout: 15000 }).catch(() => {}), + addRepositoryDialog + .waitFor({ state: 'visible', timeout: 15000 }) + .catch(() => {}), + addButton.waitFor({ state: 'visible', timeout: 15000 }).catch(() => {}), + ]) + + await failIfAppErrorDialogIsVisible(page) + + if (!(await repoFile.isVisible().catch(() => false))) { + if (!(await addRepositoryDialog.isVisible().catch(() => false))) { + await addButton.click() + } + + await addRepositoryDialog.waitFor({ state: 'visible', timeout: 15000 }) + const pathInput = addRepositoryDialog.locator( + 'input[placeholder="repository path"]' + ) + await pathInput.waitFor({ state: 'visible', timeout: 15000 }) + if ((await pathInput.inputValue()) !== smokeRepoPath) { + await pathInput.fill(smokeRepoPath) + } + await addRepositoryDialog + .locator( + 'button:has-text("Add Repository"), button:has-text("Add repository")' + ) + .click() + } + + await repoFile.waitFor({ state: 'visible', timeout: 15000 }) + await repoFile.click() + + // ── Diff ──────────────────────────────────────────────────────── + const diffContainer = page.locator('.diff-container') + await diffContainer.waitFor({ state: 'visible', timeout: 15000 }) + await expect(diffContainer).toContainText(smokeRepoFileContents, { + timeout: 15000, + }) + + // ── Commit ────────────────────────────────────────────────────── + const commitButton = page.locator( + '[aria-label="Create commit"] .commit-button' + ) + await commitButton.waitFor({ state: 'visible', timeout: 15000 }) + await dismissMoveToApplicationsDialog(page) + await commitButton.click() + + await expect + .poll(() => getSmokeRepoHeadMessage(), { timeout: 15000 }) + .toBe(`Create ${smokeRepoFileName}`) + await expect.poll(() => getSmokeRepoStatus(), { timeout: 15000 }).toBe('') + + // ── Create branch ─────────────────────────────────────────────── + const initialBranch = getSmokeRepoCurrentBranch() + const smokeBranch = 'smoke-branch' + + await dismissMoveToApplicationsDialog(page) + await page.locator('.branch-button button').click() + + const newBranchBtn = page.locator('.new-branch-button') + await newBranchBtn.waitFor({ state: 'visible', timeout: 15000 }) + await newBranchBtn.click() + + const createBranchDialog = page.locator('#create-branch') + await createBranchDialog.waitFor({ state: 'visible', timeout: 15000 }) + const branchNameInput = createBranchDialog.locator('input').first() + await branchNameInput.evaluate((el, value) => { + const input = el as HTMLInputElement + Object.getOwnPropertyDescriptor( + HTMLInputElement.prototype, + 'value' + )?.set?.call(input, value) + input.dispatchEvent(new Event('input', { bubbles: true })) + input.dispatchEvent(new Event('change', { bubbles: true })) + }, smokeBranch) + + await createBranchDialog + .locator( + 'button:has-text("Create Branch"), button:has-text("Create branch")' + ) + .click() + + await expect + .poll(() => getSmokeRepoCurrentBranch(), { timeout: 15000 }) + .toBe(smokeBranch) + + // ── Switch back ───────────────────────────────────────────────── + await dismissMoveToApplicationsDialog(page) + await page.locator('.branch-button button').click() + + await page + .locator( + `//div[contains(@class, "branches-list-item")]//div[contains(@class, "name") and normalize-space()="${initialBranch}"]` + ) + .click() + + await expect + .poll(() => getSmokeRepoCurrentBranch(), { timeout: 15000 }) + .toBe(initialBranch) + await expect.poll(() => getSmokeRepoStatus(), { timeout: 15000 }).toBe('') + }) +}) + +// ── Auto-update tests ─────────────────────────────────────────────── + +test.describe('Auto-update', () => { + test.skip( + process.platform === 'win32' && !process.env.DESKTOP_E2E_APP_PATH, + 'Windows auto-update requires an installed Squirrel app, not a packaged app directory.' + ) + + test.describe('startup update check', () => { + test('sends an update check to the mock server on launch', async ({ + mockServer, + }) => { + await expect + .poll( + async () => { + const reqs = await getMockRequests() + return reqs.some(r => isMockUpdateRequest(r.url)) + }, + { timeout: 30000, intervals: [1000] } + ) + .toBe(true) + }) + + test('does not show update banner when no update is available', async ({ + mainWindow: page, + }) => { + const banner = page.locator('#update-available') + await expect(banner).not.toBeVisible() + }) + }) + + test.describe('About dialog', () => { + test('shows the current version', async ({ mainWindow: page }) => { + await page.evaluate(() => { + require('electron').ipcRenderer.emit('menu-event', {}, 'show-about') + }) + + const aboutDialog = page.locator('#about') + await aboutDialog.waitFor({ state: 'visible', timeout: 5000 }) + + const versionText = await aboutDialog + .locator('.selectable-text') + .textContent() + expect(versionText).toMatch(/Version \d+\.\d+\.\d+/) + }) + + test('shows up-to-date status after no-update check', async ({ + mainWindow: page, + }) => { + const updateStatus = page.locator('#about .update-status') + await updateStatus.waitFor({ state: 'visible', timeout: 10000 }) + + const txt = await updateStatus.textContent() + expect(txt?.toLowerCase()).toContain('you have the latest version') + }) + + test('closes the About dialog', async ({ mainWindow: page }) => { + await page.locator('#about button[type="submit"]').click() + await page.locator('#about').waitFor({ state: 'hidden', timeout: 5000 }) + }) + }) + + test.describe('update available', () => { + test('switches mock server to return an update', async ({}) => { + await controlMockServer('reset-requests') + await controlMockServer('set-behavior/update-available') + const behavior = await controlMockServer('behavior') + expect(behavior).toBe('update-available') + }) + + test('triggers an update check and the app processes it', async ({ + mainWindow: page, + }) => { + await page.evaluate(() => { + require('electron').ipcRenderer.emit('menu-event', {}, 'show-about') + }) + + const aboutDialog = page.locator('#about') + await aboutDialog.waitFor({ state: 'visible', timeout: 5000 }) + + const checkBtn = aboutDialog.locator( + 'button.button-component:has-text("Check for Updates")' + ) + if ( + (await checkBtn.isVisible().catch(() => false)) && + (await checkBtn.isEnabled().catch(() => false)) + ) { + await checkBtn.click() + } + + // Wait for status change + const updateStatus = aboutDialog.locator('.update-status') + await expect + .poll( + async () => { + if (!(await updateStatus.isVisible().catch(() => false))) { + return '' + } + return ((await updateStatus.textContent()) ?? '').toLowerCase() + }, + { timeout: 15000, intervals: [1000] } + ) + .toMatch(/checking|downloading|ready to be installed/) + + // Close dialog + await page.locator('#about button[type="submit"]').click() + await aboutDialog + .waitFor({ state: 'hidden', timeout: 5000 }) + .catch(() => {}) + }) + + test('sent update check requests to the mock server', async ({}) => { + await expect + .poll( + async () => { + const reqs = await getMockRequests() + return reqs.filter( + r => r.method === 'GET' && isMockUpdateRequest(r.url) + ).length + }, + { timeout: 15000, intervals: [1000] } + ) + .toBeGreaterThanOrEqual(1) + }) + + test('shows installing-update warning when quitting during download', async ({ + mainWindow: page, + }) => { + await page.evaluate(() => { + require('electron').ipcRenderer.emit('menu-event', {}, 'show-about') + }) + + const aboutDialog = page.locator('#about') + await aboutDialog.waitFor({ state: 'visible', timeout: 5000 }) + + const checkBtn = aboutDialog.locator( + 'button.button-component:has-text("Check for Updates")' + ) + if ( + (await checkBtn.isVisible().catch(() => false)) && + (await checkBtn.isEnabled().catch(() => false)) + ) { + await checkBtn.click() + } + + const updateStatus = aboutDialog.locator('.update-status') + await expect + .poll( + async () => { + if (!(await updateStatus.isVisible().catch(() => false))) { + return '' + } + + return ((await updateStatus.textContent()) ?? '').toLowerCase() + }, + { timeout: 15000, intervals: [1000] } + ) + .toContain('downloading update') + + await page.locator('#about button[type="submit"]').click() + await aboutDialog + .waitFor({ state: 'hidden', timeout: 5000 }) + .catch(() => {}) + + await page.evaluate(() => { + require('electron').ipcRenderer.send('quit-app') + }) + + const dialog = page.locator('#installing-update') + await dialog.waitFor({ state: 'visible', timeout: 5000 }) + + await expect(dialog.locator('.updating-message')).toContainText( + 'Do not close GitHub Desktop while the update is in progress' + ) + + // Reset mock and trigger quit again to test Quit Anyway + await controlMockServer('set-behavior/no-update') + await controlMockServer('reset-requests') + + await page.evaluate(() => { + require('electron').ipcRenderer.send('quit-app') + }) + + const quitBtn = dialog.locator( + '.button-group.destructive button[type="button"]' + ) + await quitBtn.waitFor({ state: 'visible', timeout: 5000 }) + + // Save the trace now — the next click will kill the app and make + // the browser context unavailable for the fixture teardown. + const tracePath = require('path').join( + __dirname, + '..', + '..', + '..', + 'playwright-videos', + `trace-${Date.now()}.zip` + ) + await page + .context() + .tracing.stop({ path: tracePath }) + .catch(() => {}) + + // Get PID before quitting so we can verify the process exits + const rendererPid: number = await page.evaluate(() => process.pid) + + await quitBtn.click() + + // Poll the OS to confirm the renderer process exited + await expect + .poll( + () => { + try { + process.kill(rendererPid, 0) + return false + } catch { + return true + } + }, + { timeout: 10000, intervals: [200] } + ) + .toBe(true) + + terminateWindowsUpdaterProcesses() + }) + }) +}) diff --git a/app/test/e2e/e2e-fixtures.ts b/app/test/e2e/e2e-fixtures.ts new file mode 100644 index 00000000000..92b470be6cd --- /dev/null +++ b/app/test/e2e/e2e-fixtures.ts @@ -0,0 +1,218 @@ +/* eslint-disable no-sync */ + +/** + * Shared Playwright fixtures for GitHub Desktop e2e tests. + * + * Provides: + * - `app` — the ElectronApplication instance + * - `page` — the main BrowserWindow page + * - `mockServer` — the mock update server (with control helpers) + * + * All fixtures are scoped to the **worker** so the app launches once + * and all tests in the file share the same session (one Electron + * session runs all specs sequentially). + */ + +import fs from 'fs' +import http from 'http' +import os from 'os' +import path from 'path' +import { spawnSync } from 'child_process' +import { + test as base, + type ElectronApplication, + type Page, +} from '@playwright/test' +import { _electron as electron } from 'playwright' +import { ensureSmokeTestRepository, smokeRepoPath } from './test-helpers' +import { + createMockUpdateServer, + MOCK_CONTROL_URL, + type IMockUpdateServer, +} from './mock-update-server' +import { getDistPath, getExecutableName } from '../../../script/dist-info' +import { getProductName } from '../../package-info' + +const projectRoot = path.resolve(__dirname, '..', '..', '..') +const userDataDir = path.join(os.tmpdir(), 'github-desktop-pw-e2e') +const fakeHomeDir = path.join(os.tmpdir(), 'github-desktop-pw-fake-home') +const installedAppExecutablePath = process.env.DESKTOP_E2E_APP_PATH + +function getPackagedAppExecutablePath() { + const distPath = getDistPath() + + if (process.platform === 'darwin') { + const productName = getProductName() + return path.join( + distPath, + `${productName}.app`, + 'Contents', + 'MacOS', + productName + ) + } + + if (process.platform === 'win32') { + return path.join(distPath, `${getExecutableName()}.exe`) + } + + return path.join(distPath, getExecutableName()) +} + +const e2eAppExecutablePath = + installedAppExecutablePath ?? getPackagedAppExecutablePath() + +function killLingeringWindowsUpdaterProcesses() { + if (process.platform !== 'win32') { + return + } + + for (const imageName of ['Update.exe', 'GitHubDesktop.exe']) { + spawnSync('taskkill', ['/F', '/T', '/IM', imageName], { + stdio: 'ignore', + windowsHide: true, + }) + } +} + +export function terminateWindowsUpdaterProcesses() { + killLingeringWindowsUpdaterProcesses() +} + +// ── Helpers exposed to tests ──────────────────────────────────────── + +export function controlMockServer(action: string): Promise { + return new Promise((resolve, reject) => { + http + .get(`${MOCK_CONTROL_URL}/${action}`, res => { + let data = '' + res.on('data', (chunk: string) => (data += chunk)) + res.on('end', () => resolve(data)) + }) + .on('error', reject) + }) +} + +export async function getMockRequests(): Promise< + ReadonlyArray<{ method: string; url: string }> +> { + return JSON.parse(await controlMockServer('requests')) +} + +export async function dismissMoveToApplicationsDialog(page: Page) { + const btn = page.locator( + 'button:has-text("Not Now"), button:has-text("Not now")' + ) + if (await btn.isVisible({ timeout: 2000 }).catch(() => false)) { + await btn.click() + await btn.waitFor({ state: 'hidden', timeout: 10000 }).catch(() => {}) + } +} + +// ── Fixtures ──────────────────────────────────────────────────────── + +type E2EFixtures = { + app: ElectronApplication + mainWindow: Page + mockServer: IMockUpdateServer +} + +export const test = base.extend<{}, E2EFixtures>({ + // Worker-scoped: one Electron app per test file. + // Depends on mockServer so the update server is ready before launch. + app: [ + async ({ mockServer }, use) => { + // Setup directories + ensureSmokeTestRepository() + + if (!fs.existsSync(e2eAppExecutablePath)) { + throw new Error( + `E2E app not found at ${e2eAppExecutablePath}. Run yarn test:e2e:build first.` + ) + } + + fs.rmSync(userDataDir, { recursive: true, force: true }) + fs.mkdirSync(userDataDir, { recursive: true }) + fs.rmSync(fakeHomeDir, { recursive: true, force: true }) + fs.mkdirSync(fakeHomeDir, { recursive: true }) + + const app = await electron.launch({ + executablePath: e2eAppExecutablePath, + args: [`--user-data-dir=${userDataDir}`, `--cli-open=${smokeRepoPath}`], + env: { + ...process.env, + GIT_CONFIG_GLOBAL: path.join(fakeHomeDir, '.gitconfig'), + GIT_CONFIG_SYSTEM: path.join(fakeHomeDir, '.gitconfig-system'), + XDG_CONFIG_HOME: path.join(fakeHomeDir, '.config'), + SSH_AUTH_SOCK: '', + GIT_SSH_COMMAND: 'false', + }, + recordVideo: { + dir: path.join(projectRoot, 'playwright-videos'), + size: { width: 1280, height: 800 }, + }, + timeout: 30000, + }) + + await use(app) + + if (process.platform === 'win32') { + killLingeringWindowsUpdaterProcesses() + } + + await app.close().catch(() => {}) + killLingeringWindowsUpdaterProcesses() + await new Promise(resolve => setTimeout(resolve, 1000)) + }, + { scope: 'worker' }, + ], + + mainWindow: [ + async ({ app }, use) => { + const page = await app.firstWindow() + + page.on('console', message => { + const text = message.text() + if (message.type() === 'error' || text.includes('Uncaught exception')) { + console.log(`[e2e:console:${message.type()}] ${text}`) + } + }) + + page.on('pageerror', error => { + const details = error.stack ?? error.message + console.log(`[e2e:pageerror] ${details}`) + }) + + // Start tracing for this worker session + await page.context().tracing.start({ + screenshots: true, + snapshots: true, + }) + + await use(page) + + // Save trace on teardown + const tracePath = path.join( + projectRoot, + 'playwright-videos', + `trace-${Date.now()}.zip` + ) + await page + .context() + .tracing.stop({ path: tracePath }) + .catch(() => {}) + }, + { scope: 'worker' }, + ], + + mockServer: [ + async ({}, use) => { + const server = await createMockUpdateServer() + await use(server) + await server.close() + }, + { scope: 'worker' }, + ], +}) + +export { expect } from '@playwright/test' diff --git a/app/test/e2e/mock-update-server.ts b/app/test/e2e/mock-update-server.ts new file mode 100644 index 00000000000..289f0c4c6e8 --- /dev/null +++ b/app/test/e2e/mock-update-server.ts @@ -0,0 +1,218 @@ +/* eslint-disable no-sync */ + +import http from 'http' +import type net from 'net' +import { + getWindowsFullNugetPackageName, + getWindowsIdentifierName, +} from '../../../script/dist-info' + +/** Fixed port used for the mock update server during e2e tests. */ +export const MOCK_UPDATE_PORT = 51789 +export const MOCK_UPDATE_URL = `http://127.0.0.1:${MOCK_UPDATE_PORT}/update` + +/** + * URL that e2e tests can use to control the mock server behaviour at runtime + * via simple GET requests (e.g. `/_control/set-behavior/update-available`). + */ +export const MOCK_CONTROL_URL = `http://127.0.0.1:${MOCK_UPDATE_PORT}/_control` + +const currentWindowsPackageName = getWindowsFullNugetPackageName() +const nextWindowsPackageName = `${getWindowsIdentifierName()}-99.0.0-full.nupkg` +const fakeSha = '0123456789012345678901234567890123456789' +const fakePackageSize = '999999999' + +function isWindowsFeedRequest(url: string) { + return url.includes('/RELEASES') || url.endsWith('.nupkg') +} + +function getWindowsNoUpdateReleases() { + return `${fakeSha} ${currentWindowsPackageName} ${fakePackageSize}` +} + +function getWindowsUpdateAvailableReleases() { + return `${fakeSha} ${nextWindowsPackageName} ${fakePackageSize}` +} + +type UpdateBehavior = 'no-update' | 'update-available' + +export interface IMockUpdateServer { + readonly server: http.Server + readonly url: string + + /** All requests received by the mock server (excluding control requests). */ + readonly requests: Array<{ method: string; url: string }> + + /** Change how the server responds to update checks. */ + setBehavior(behavior: UpdateBehavior): void + + /** Reset the captured request log. */ + resetRequests(): void + + close(): Promise +} + +/** + * Create a mock update server that mimics the responses from + * central.github.com for Squirrel (macOS) and Squirrel.Windows. + * + * By default, it responds with "no update available" (HTTP 204). + * + * In `update-available` mode, the JSON feed tells Squirrel an update + * exists. Squirrel will attempt to download the zip, receive a 404, and + * emit an `error` event. This is enough to verify that the app correctly + * processes the update feed and transitions through the expected states. + * + * Full binary verification is not possible in dev builds because + * Squirrel.Mac requires the update zip to satisfy the running app's code + * signing designated requirements — something only production-signed + * builds can provide. + */ +export function createMockUpdateServer(): Promise { + return new Promise((resolve, reject) => { + let behavior: UpdateBehavior = 'no-update' + const requests: Array<{ method: string; url: string }> = [] + const sockets = new Set() + + const server = http.createServer((req, res) => { + const url = req.url ?? '/' + + // ── Control plane ───────────────────────────────────────────── + if (url.startsWith('/_control/')) { + const action = url.replace('/_control/', '') + + if (action === 'set-behavior/no-update') { + behavior = 'no-update' + res.writeHead(200) + res.end('ok') + return + } + + if (action === 'set-behavior/update-available') { + behavior = 'update-available' + res.writeHead(200) + res.end('ok') + return + } + + if (action === 'reset-requests') { + requests.length = 0 + res.writeHead(200) + res.end('ok') + return + } + + if (action === 'requests') { + res.writeHead(200, { 'content-type': 'application/json' }) + res.end(JSON.stringify(requests)) + return + } + + if (action === 'behavior') { + res.writeHead(200) + res.end(behavior) + return + } + + res.writeHead(404) + res.end('unknown control action') + return + } + + // ── Update plane ────────────────────────────────────────────── + requests.push({ method: req.method ?? 'GET', url }) + + // Serve fake download URLs by hanging forever — send headers but never + // finish the body. This keeps the updater in "downloading" state + // without ever completing or failing validation. + if (url.startsWith('/download/') || url.endsWith('.nupkg')) { + res.writeHead(200, { + 'content-type': 'application/octet-stream', + 'content-length': '999999999', + }) + // Intentionally never call res.end() — the connection stays + // open until the server is shut down or the client disconnects. + return + } + + if (req.method === 'HEAD') { + // Priority update status check. + res.writeHead(200, { 'x-prioritize-update': 'false' }) + res.end() + return + } + + if (isWindowsFeedRequest(url)) { + const body = + behavior === 'update-available' + ? getWindowsUpdateAvailableReleases() + : getWindowsNoUpdateReleases() + + if (url.includes('/RELEASES')) { + res.writeHead(200, { + 'content-type': 'text/plain; charset=utf-8', + 'content-length': Buffer.byteLength(body), + }) + res.end(body) + return + } + } + + if (behavior === 'no-update') { + res.writeHead(204) + res.end() + return + } + + if (behavior === 'update-available') { + // Squirrel.Mac JSON feed. The download URL points back to this server's + // /download/ handler which hangs forever, keeping the app in + // "downloading" state without completing or erroring. + const body = JSON.stringify({ + url: `http://127.0.0.1:${MOCK_UPDATE_PORT}/download/update.zip`, + name: '99.0.0', + notes: 'E2E test update', + pub_date: new Date().toISOString(), + }) + res.writeHead(200, { + 'content-type': 'application/json', + 'content-length': Buffer.byteLength(body), + }) + res.end(body) + return + } + + res.writeHead(204) + res.end() + }) + + server.on('error', reject) + server.on('connection', socket => { + sockets.add(socket) + socket.on('close', () => sockets.delete(socket)) + }) + server.listen(MOCK_UPDATE_PORT, '127.0.0.1', () => { + const instance: IMockUpdateServer = { + server, + url: MOCK_UPDATE_URL, + requests, + setBehavior(b: UpdateBehavior) { + behavior = b + }, + resetRequests() { + requests.length = 0 + }, + close() { + return new Promise((res, rej) => { + for (const socket of sockets) { + socket.destroy() + } + + server.close(err => (err ? rej(err) : res())) + }) + }, + } + resolve(instance) + }) + }) +} diff --git a/app/test/e2e/playwright.config.ts b/app/test/e2e/playwright.config.ts new file mode 100644 index 00000000000..d131a06d286 --- /dev/null +++ b/app/test/e2e/playwright.config.ts @@ -0,0 +1,22 @@ +/* eslint-disable no-sync */ + +import path from 'path' +import { defineConfig } from '@playwright/test' + +const projectRoot = path.resolve(__dirname, '..', '..', '..') + +// eslint-disable-next-line no-restricted-syntax +export default defineConfig({ + testDir: __dirname, + testMatch: '*.e2e.ts', + timeout: 120_000, + retries: 0, + workers: 1, + + outputDir: path.join(projectRoot, 'playwright-videos'), + + // Video recording and tracing are configured in the Electron- + // specific fixtures (see e2e-fixtures.ts) rather than here, + // because @playwright/test `use.video` / `use.trace` only apply + // to browser contexts, not Electron apps. +}) diff --git a/app/test/e2e/test-helpers.ts b/app/test/e2e/test-helpers.ts new file mode 100644 index 00000000000..7dd5c3e593d --- /dev/null +++ b/app/test/e2e/test-helpers.ts @@ -0,0 +1,64 @@ +/* eslint-disable no-sync */ + +import { execFileSync } from 'child_process' +import fs from 'fs' +import os from 'os' +import path from 'path' + +export const smokeRepoPath = path.join( + os.tmpdir(), + 'github-desktop-e2e-smoke-repository' +) +export const smokeRepoName = path.basename(smokeRepoPath) +export const smokeRepoFileName = 'smoke-change.txt' +export const smokeRepoFileContents = + 'This file should appear in the changes list.' + +export function ensureSmokeTestRepository() { + fs.rmSync(smokeRepoPath, { recursive: true, force: true }) + fs.mkdirSync(smokeRepoPath, { recursive: true }) + + runGit(['init'], smokeRepoPath) + runGit(['config', 'user.name', 'GitHub Desktop E2E'], smokeRepoPath) + runGit(['config', 'user.email', 'desktop-e2e@example.com'], smokeRepoPath) + + fs.writeFileSync( + path.join(smokeRepoPath, 'README.md'), + '# GitHub Desktop Smoke Repo\n' + ) + + runGit(['add', 'README.md'], smokeRepoPath) + runGit(['commit', '-m', 'Initial commit'], smokeRepoPath) + + fs.writeFileSync( + path.join(smokeRepoPath, smokeRepoFileName), + `${smokeRepoFileContents}\n` + ) +} + +function runGit(args: ReadonlyArray, cwd: string) { + execFileSync('git', [...args], { + cwd, + stdio: 'ignore', + }) +} + +function readGitOutput(args: ReadonlyArray, cwd: string) { + return execFileSync('git', [...args], { + cwd, + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'ignore'], + }).trim() +} + +export function getSmokeRepoStatus() { + return readGitOutput(['status', '--short'], smokeRepoPath) +} + +export function getSmokeRepoHeadMessage() { + return readGitOutput(['log', '-1', '--pretty=%s'], smokeRepoPath) +} + +export function getSmokeRepoCurrentBranch() { + return readGitOutput(['branch', '--show-current'], smokeRepoPath) +} diff --git a/app/test/globals.mts b/app/test/globals.mts index b33fab090c4..d57bee04fd1 100644 --- a/app/test/globals.mts +++ b/app/test/globals.mts @@ -40,9 +40,62 @@ Object.assign(globalThis, { BroadcastChannel: undefined, }) +Object.assign(globalThis, { + Event: window.Event, + CustomEvent: window.CustomEvent, +}) + +type DialogElement = HTMLElement & { + open?: boolean + showModal?: () => void + close?: () => void +} + +const dialogPrototype = HTMLElement.prototype as DialogElement + +if (typeof dialogPrototype.showModal !== 'function') { + dialogPrototype.showModal = function () { + this.open = true + this.setAttribute('open', '') + } +} + +if (typeof dialogPrototype.close !== 'function') { + dialogPrototype.close = function () { + this.open = false + this.removeAttribute('open') + } +} + +if (globalThis.ResizeObserver === undefined) { + class TestResizeObserver { + public observe() {} + public disconnect() {} + } + + Object.assign(globalThis, { ResizeObserver: TestResizeObserver }) +} + +Object.defineProperty(HTMLFormElement.prototype, 'requestSubmit', { + configurable: true, + writable: true, + value: function () { + this.dispatchEvent( + new window.Event('submit', { bubbles: true, cancelable: true }) + ) + }, +}) + mock.module('electron', { namedExports: { shell: {}, - ipcRenderer: { on: mock.fn(x => {}) }, + ipcRenderer: { + on: mock.fn(() => {}), + once: mock.fn(() => {}), + send: mock.fn(() => {}), + sendSync: mock.fn(() => {}), + invoke: mock.fn(async () => undefined), + removeListener: mock.fn(() => {}), + }, }, }) diff --git a/package.json b/package.json index df171b7f83b..63a8baf6833 100644 --- a/package.json +++ b/package.json @@ -12,6 +12,9 @@ "test": "node script/test.mjs", "test:unit": "node script/test.mjs", "test:setup": "ts-node -P script/tsconfig.json script/test-setup.ts", + "test:e2e": "yarn test:e2e:build && yarn test:e2e:run", + "test:e2e:build": "cross-env DESKTOP_E2E_UPDATES_URL=http://127.0.0.1:51789/update NODE_ENV=production RELEASE_CHANNEL=production DESKTOP_E2E=1 yarn build:prod", + "test:e2e:run": "npx playwright test --config app/test/e2e/playwright.config.ts", "postinstall": "ts-node -P script/tsconfig.json script/post-install.ts", "start": "cross-env NODE_ENV=development ts-node -P script/tsconfig.json script/start.ts", "start:prod": "cross-env NODE_ENV=production ts-node -P script/tsconfig.json script/start.ts", @@ -101,6 +104,7 @@ }, "devDependencies": { "@github/markdownlint-github": "^0.1.0", + "@playwright/test": "^1.58.2", "@types/byline": "^4.2.31", "@types/classnames": "^2.2.2", "@types/codemirror": "^5.60.15", @@ -152,6 +156,7 @@ "eslint-plugin-github": "^5.1.5", "markdownlint-cli": "^0.32.2", "node-test-github-reporter": "^1.2.0", + "playwright": "^1.58.2", "reserved-words": "^0.1.2", "tsconfig-paths": "^3.9.0", "tsx": "^4.19.3", diff --git a/script/playwright-electron.ts b/script/playwright-electron.ts new file mode 100644 index 00000000000..265e582990b --- /dev/null +++ b/script/playwright-electron.ts @@ -0,0 +1,110 @@ +/** + * Launch GitHub Desktop via Playwright for agent-driven UI verification. + * + * This script starts the app and keeps it running so that an agent can + * interact with it using Playwright browser automation tools (click + * elements, take screenshots, inspect the DOM, etc.). + * + * Set RECORD_VIDEO=1 to enable video recording of the session. The video + * is saved to the project root when the app closes. + * + * Usage: + * npx ts-node -P script/tsconfig.json script/playwright-electron.ts + * RECORD_VIDEO=1 npx ts-node -P script/tsconfig.json script/playwright-electron.ts + * + * Prerequisites: + * yarn build:dev # or yarn compile:dev — need out/main.js + */ + +/* eslint-disable no-sync */ + +import path from 'path' +import fs from 'fs' +import os from 'os' +import { _electron as electron } from 'playwright' + +const projectRoot = path.resolve(__dirname, '..') +const appEntryPoint = path.join(projectRoot, 'out', 'main.js') +const userDataDir = path.join(os.tmpdir(), 'github-desktop-playwright-verify') +const videosDir = path.join(projectRoot, 'playwright-videos') + +if (!fs.existsSync(appEntryPoint)) { + console.error( + `Error: Built app not found at ${appEntryPoint}\nRun 'yarn build:dev' or 'yarn compile:dev' first.` + ) + process.exit(1) +} + +fs.rmSync(userDataDir, { recursive: true, force: true }) +fs.mkdirSync(userDataDir, { recursive: true }) + +const recordVideo = process.env.RECORD_VIDEO === '1' +const repoPath = process.argv[2] // Optional: path to a repo to open + +async function main() { + if (recordVideo) { + fs.mkdirSync(videosDir, { recursive: true }) + console.log( + `Video recording enabled — videos will be saved to ${videosDir}` + ) + } + + const appArgs = [appEntryPoint, `--user-data-dir=${userDataDir}`] + if (repoPath) { + appArgs.push(`--cli-open=${repoPath}`) + console.log(`Opening repository: ${repoPath}`) + } + + console.log('Launching GitHub Desktop via Playwright…') + const app = await electron.launch({ + args: appArgs, + env: { + ...process.env, + // Isolate all user config so the agent doesn't read or modify the + // developer's real settings. + GIT_CONFIG_GLOBAL: path.join(userDataDir, '.gitconfig'), + GIT_CONFIG_SYSTEM: path.join(userDataDir, '.gitconfig-system'), + XDG_CONFIG_HOME: path.join(userDataDir, '.config'), + // Prevent SSH from using the developer's real keys + SSH_AUTH_SOCK: '', + GIT_SSH_COMMAND: 'false', + }, + recordVideo: recordVideo + ? { dir: videosDir, size: { width: 1280, height: 800 } } + : undefined, + timeout: 30000, + }) + + const window = await app.firstWindow() + console.log(`Window title: ${await window.title()}`) + console.log(`Window URL: ${window.url()}`) + console.log('\nPlaywright Electron session is active.') + console.log( + 'Use Playwright browser automation tools to interact with the app.' + ) + console.log('Press Ctrl+C to close.\n') + + // When the process is interrupted, close gracefully so videos are flushed + process.on('SIGINT', async () => { + console.log('\nClosing app…') + await app.close() + + if (recordVideo) { + const video = window.video() + if (video) { + const videoPath = await video.path() + console.log(`Video saved: ${videoPath}`) + } + } + + process.exit(0) + }) + + // Keep process alive until interrupted + await new Promise(() => {}) +} + +main().catch(err => { + console.error(err) + process.exit(1) +}) diff --git a/script/post-install.ts b/script/post-install.ts index 4d35d219a80..e398c4b0294 100644 --- a/script/post-install.ts +++ b/script/post-install.ts @@ -13,6 +13,20 @@ const options: SpawnSyncOptions = { stdio: 'inherit', } +const captureOutputOptions: SpawnSyncOptions = { + cwd: root, + encoding: 'utf8', +} + +// Some Windows CI runners do not expose an `npx` executable on PATH, so +// invoke the locally installed Playwright CLI through the current Node binary. +// Resolve from the exported package root since `playwright/cli` is not exported. +const playwrightPackagePath = require.resolve('playwright/package.json') +const playwrightCliPath = Path.join( + Path.dirname(playwrightPackagePath), + 'cli.js' +) + function findYarnVersion(callback: (path: string) => void) { glob('vendor/yarn-*.js', (error, files) => { if (error != null) { @@ -53,4 +67,29 @@ findYarnVersion(path => { if (result.status !== 0) { process.exit(result.status || 1) } + + // Capture output here so CI failures include the Playwright-specific error. + result = spawnSync( + process.execPath, + [playwrightCliPath, 'install', 'ffmpeg'], + captureOutputOptions + ) + + if (result.status !== 0) { + console.error( + 'Error: failed to install Playwright ffmpeg (video recording may not work)', + '\nplatform:', + process.platform, + '\nstatus:', + result.status, + '\nsignal:', + result.signal, + '\nerror:', + result.error, + '\nstdout:', + result.stdout, + '\nstderr:', + result.stderr + ) + } }) From 6a256fad40b76c2585f7d6311dc0e7377bd1a3eb Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Thu, 19 Mar 2026 13:14:12 +0100 Subject: [PATCH 06/26] Update yarn.lock --- yarn.lock | 35 +++++++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/yarn.lock b/yarn.lock index 3cc97796127..4e6b5327d87 100644 --- a/yarn.lock +++ b/yarn.lock @@ -844,6 +844,13 @@ resolved "https://registry.yarnpkg.com/@pkgr/core/-/core-0.1.1.tgz#1ec17e2edbec25c8306d424ecfbf13c7de1aaa31" integrity sha512-cq8o4cWH0ibXh9VGi5P20Tu9XF/0fFXl9EUinr9QfTM7a7p0oTA4iJRCQWppXR1Pg8dSM0UCItCkPwsk9qWWYA== +"@playwright/test@^1.58.2": + version "1.58.2" + resolved "https://registry.yarnpkg.com/@playwright/test/-/test-1.58.2.tgz#b0ad585d2e950d690ef52424967a42f40c6d2cbd" + integrity sha512-akea+6bHYBBfA9uQqSYmlJXn61cTa+jbO87xVLCWbTqbWadRVmhxlXATaOjOgcBaWU4ePo0wB41KMFv3o35IXA== + dependencies: + playwright "1.58.2" + "@polka/url@^1.0.0-next.24": version "1.0.0-next.25" resolved "https://registry.yarnpkg.com/@polka/url/-/url-1.0.0-next.25.tgz#f077fdc0b5d0078d30893396ff4827a13f99e817" @@ -3716,6 +3723,11 @@ fs.realpath@^1.0.0: resolved "https://registry.yarnpkg.com/fs.realpath/-/fs.realpath-1.0.0.tgz#1504ad2523158caa40db4a2787cb01411994ea4f" integrity sha1-FQStJSMVjKpA20onh8sBQRmU6k8= +fsevents@2.3.2: + version "2.3.2" + resolved "https://registry.yarnpkg.com/fsevents/-/fsevents-2.3.2.tgz#8a526f78b8fdf4623b709e0b975c52c24c02fd1a" + integrity sha512-xiqMQR4xAeHTuB9uWm+fFRcIOgKBMiOBP+eXiyT7jsgVCq1bkVygt00oASowB7EdtpOHaaPgKt812P9ab+DDKA== + fsevents@~2.1.2: version "2.1.3" resolved "https://registry.yarnpkg.com/fsevents/-/fsevents-2.1.3.tgz#fb738703ae8d2f9fe900c33836ddebee8b97f23e" @@ -5738,6 +5750,20 @@ pkg-dir@^4.2.0: dependencies: find-up "^4.0.0" +playwright-core@1.58.2: + version "1.58.2" + resolved "https://registry.yarnpkg.com/playwright-core/-/playwright-core-1.58.2.tgz#ac5f5b4b10d29bcf934415f0b8d133b34b0dcb13" + integrity sha512-yZkEtftgwS8CsfYo7nm0KE8jsvm6i/PTgVtB8DL726wNf6H2IMsDuxCpJj59KDaxCtSnrWan2AeDqM7JBaultg== + +playwright@1.58.2, playwright@^1.58.2: + version "1.58.2" + resolved "https://registry.yarnpkg.com/playwright/-/playwright-1.58.2.tgz#afe547164539b0bcfcb79957394a7a3fa8683cfd" + integrity sha512-vA30H8Nvkq/cPBnNw4Q8TWz1EJyqgpuinBcHET0YVJVFldr8JDNiU9LaWAE1KqSkRYazuaBhTpB5ZzShOezQ6A== + dependencies: + playwright-core "1.58.2" + optionalDependencies: + fsevents "2.3.2" + plist@^3.0.0, plist@^3.0.4, plist@^3.0.5: version "3.0.6" resolved "https://registry.yarnpkg.com/plist/-/plist-3.0.6.tgz#7cfb68a856a7834bca6dbfe3218eb9c7740145d3" @@ -6578,14 +6604,7 @@ string.prototype.trimstart@^1.0.7: define-properties "^1.2.0" es-abstract "^1.22.1" -"strip-ansi-cjs@npm:strip-ansi@^6.0.1": - version "6.0.1" - resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" - integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== - dependencies: - ansi-regex "^5.0.1" - -strip-ansi@^6.0.0, strip-ansi@^6.0.1: +"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.0, strip-ansi@^6.0.1: version "6.0.1" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== From fb50414556f57c0d9e135b001b1d129b997c32c6 Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Thu, 19 Mar 2026 13:42:25 +0100 Subject: [PATCH 07/26] E2E: support unpackaged mode and skip packaging Add support for running Playwright E2E tests against an 'unpackaged' app and make packaged the default in CI. Updates: CI workflow runs packaged E2E step; .gitignore ignores playwright-videos; e2e fixtures gain DESKTOP_E2E_APP_MODE handling and unified launch option logic; package.json adds distinct packaged/unpackaged build/run scripts; build script honors DESKTOP_SKIP_PACKAGE to skip packaging when building unpackaged tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/ci.yml | 4 ++-- .gitignore | 1 + app/test/e2e/e2e-fixtures.ts | 42 ++++++++++++++++++++++++++++++++---- package.json | 12 ++++++++--- script/build.ts | 6 ++++++ 5 files changed, 56 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c590555843e..e539e78af67 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -296,5 +296,5 @@ jobs: } Add-Content -Path $env:GITHUB_ENV -Value "DESKTOP_E2E_APP_PATH=$installedExe" - - name: Run E2E smoke tests - run: yarn test:e2e:run + - name: Run packaged E2E smoke tests + run: yarn test:e2e:run:packaged diff --git a/.gitignore b/.gitignore index 3608eb085d2..fbf84e19879 100644 --- a/.gitignore +++ b/.gitignore @@ -19,3 +19,4 @@ vendor/desktop-trampoline/build/ junit*.xml *.swp tslint-rules/ +playwright-videos/ diff --git a/app/test/e2e/e2e-fixtures.ts b/app/test/e2e/e2e-fixtures.ts index 92b470be6cd..147d0398baa 100644 --- a/app/test/e2e/e2e-fixtures.ts +++ b/app/test/e2e/e2e-fixtures.ts @@ -37,6 +37,12 @@ const projectRoot = path.resolve(__dirname, '..', '..', '..') const userDataDir = path.join(os.tmpdir(), 'github-desktop-pw-e2e') const fakeHomeDir = path.join(os.tmpdir(), 'github-desktop-pw-fake-home') const installedAppExecutablePath = process.env.DESKTOP_E2E_APP_PATH +// `packaged` is the default because CI runs the suite against packaged or +// installed production artifacts. `unpackaged` exists for local iteration so +// the same tests can run against the staged app in `out/main.js` without +// requiring packaging or signing. +const e2eAppMode = process.env.DESKTOP_E2E_APP_MODE ?? 'packaged' +const unpackagedAppEntryPoint = path.join(projectRoot, 'out', 'main.js') function getPackagedAppExecutablePath() { const distPath = getDistPath() @@ -62,6 +68,33 @@ function getPackagedAppExecutablePath() { const e2eAppExecutablePath = installedAppExecutablePath ?? getPackagedAppExecutablePath() +function getE2ELaunchOptions() { + if (installedAppExecutablePath !== undefined) { + return { + executablePath: installedAppExecutablePath, + args: [`--user-data-dir=${userDataDir}`, `--cli-open=${smokeRepoPath}`], + missingPath: installedAppExecutablePath, + } + } + + if (e2eAppMode === 'unpackaged') { + return { + args: [ + unpackagedAppEntryPoint, + `--user-data-dir=${userDataDir}`, + `--cli-open=${smokeRepoPath}`, + ], + missingPath: unpackagedAppEntryPoint, + } + } + + return { + executablePath: e2eAppExecutablePath, + args: [`--user-data-dir=${userDataDir}`, `--cli-open=${smokeRepoPath}`], + missingPath: e2eAppExecutablePath, + } +} + function killLingeringWindowsUpdaterProcesses() { if (process.platform !== 'win32') { return @@ -125,9 +158,11 @@ export const test = base.extend<{}, E2EFixtures>({ // Setup directories ensureSmokeTestRepository() - if (!fs.existsSync(e2eAppExecutablePath)) { + const launchOptions = getE2ELaunchOptions() + + if (!fs.existsSync(launchOptions.missingPath)) { throw new Error( - `E2E app not found at ${e2eAppExecutablePath}. Run yarn test:e2e:build first.` + `E2E app not found at ${launchOptions.missingPath}. Run the matching E2E build step first.` ) } @@ -137,8 +172,7 @@ export const test = base.extend<{}, E2EFixtures>({ fs.mkdirSync(fakeHomeDir, { recursive: true }) const app = await electron.launch({ - executablePath: e2eAppExecutablePath, - args: [`--user-data-dir=${userDataDir}`, `--cli-open=${smokeRepoPath}`], + ...launchOptions, env: { ...process.env, GIT_CONFIG_GLOBAL: path.join(fakeHomeDir, '.gitconfig'), diff --git a/package.json b/package.json index 63a8baf6833..800a1552a92 100644 --- a/package.json +++ b/package.json @@ -12,9 +12,15 @@ "test": "node script/test.mjs", "test:unit": "node script/test.mjs", "test:setup": "ts-node -P script/tsconfig.json script/test-setup.ts", - "test:e2e": "yarn test:e2e:build && yarn test:e2e:run", - "test:e2e:build": "cross-env DESKTOP_E2E_UPDATES_URL=http://127.0.0.1:51789/update NODE_ENV=production RELEASE_CHANNEL=production DESKTOP_E2E=1 yarn build:prod", - "test:e2e:run": "npx playwright test --config app/test/e2e/playwright.config.ts", + "test:e2e": "yarn test:e2e:packaged", + "test:e2e:packaged": "yarn test:e2e:build:packaged && yarn test:e2e:run:packaged", + "test:e2e:unpackaged": "yarn test:e2e:build:unpackaged && yarn test:e2e:run:unpackaged", + "test:e2e:build": "yarn test:e2e:build:packaged", + "test:e2e:build:packaged": "cross-env DESKTOP_E2E_UPDATES_URL=http://127.0.0.1:51789/update NODE_ENV=production RELEASE_CHANNEL=production DESKTOP_E2E=1 yarn build:prod", + "test:e2e:build:unpackaged": "cross-env DESKTOP_E2E_UPDATES_URL=http://127.0.0.1:51789/update NODE_ENV=production RELEASE_CHANNEL=production DESKTOP_SKIP_PACKAGE=1 yarn build:prod", + "test:e2e:run": "yarn test:e2e:run:packaged", + "test:e2e:run:packaged": "npx playwright test --config app/test/e2e/playwright.config.ts", + "test:e2e:run:unpackaged": "cross-env DESKTOP_E2E_APP_MODE=unpackaged npx playwright test --config app/test/e2e/playwright.config.ts", "postinstall": "ts-node -P script/tsconfig.json script/post-install.ts", "start": "cross-env NODE_ENV=development ts-node -P script/tsconfig.json script/start.ts", "start:prod": "cross-env NODE_ENV=production ts-node -P script/tsconfig.json script/start.ts", diff --git a/script/build.ts b/script/build.ts index 167eafbe44e..234ccf3b811 100755 --- a/script/build.ts +++ b/script/build.ts @@ -58,6 +58,7 @@ import assert from 'assert' const isPublishableBuild = isPublishable() const isNonProductionRelease = getChannel() !== 'production' const isDevelopmentBuild = getChannel() === 'development' +const shouldSkipPackaging = process.env.DESKTOP_SKIP_PACKAGE === '1' const projectRoot = path.join(__dirname, '..') const entitlementsSuffix = isDevelopmentBuild ? '-dev' : '' @@ -113,6 +114,11 @@ verifyInjectedSassVariables(outRoot) }) }) .then(() => { + if (shouldSkipPackaging) { + console.log('Skipping packaging…') + return [outRoot] + } + console.log('Packaging…') return packageApp() }) From 2d2a69ebc0b9f086afffc5f9c5e5fbf3a75922b8 Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Thu, 19 Mar 2026 13:45:00 +0100 Subject: [PATCH 08/26] Upload E2E artifacts in CI Add a CI step to always upload end-to-end test artifacts (playwright videos) after packaged E2E smoke tests. Uses actions/upload-artifact@v4 and names artifacts by matrix friendlyName and arch; warns if no files found. --- .github/workflows/ci.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e539e78af67..ca20e730b7b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -298,3 +298,10 @@ jobs: Add-Content -Path $env:GITHUB_ENV -Value "DESKTOP_E2E_APP_PATH=$installedExe" - name: Run packaged E2E smoke tests run: yarn test:e2e:run:packaged + - name: Upload E2E artifacts + if: ${{ always() }} + uses: actions/upload-artifact@v4 + with: + name: e2e-${{matrix.friendlyName}}-${{matrix.arch}} + path: playwright-videos/** + if-no-files-found: warn From 2655bf91581c30459d8138086176de785f6612c8 Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Thu, 19 Mar 2026 13:47:45 +0100 Subject: [PATCH 09/26] Extract CI setup into reusable actions Add two composite GitHub Actions to centralize repeated CI steps: setup-ci-environment (Python/Node/ffmpeg/install deps) and setup-windows-signing (install Azure Code Signing client + OIDC login). Update .github/workflows/ci.yml to use these reusable actions and pass the appropriate inputs, reducing duplication and making Windows signing and ffmpeg installation configurable. --- .../actions/setup-ci-environment/action.yml | 39 +++++++++++ .../actions/setup-windows-signing/action.yml | 35 ++++++++++ .github/workflows/ci.yml | 67 ++++--------------- 3 files changed, 87 insertions(+), 54 deletions(-) create mode 100644 .github/actions/setup-ci-environment/action.yml create mode 100644 .github/actions/setup-windows-signing/action.yml diff --git a/.github/actions/setup-ci-environment/action.yml b/.github/actions/setup-ci-environment/action.yml new file mode 100644 index 00000000000..564c28bafac --- /dev/null +++ b/.github/actions/setup-ci-environment/action.yml @@ -0,0 +1,39 @@ +name: Setup CI Environment +description: Set up Python, Node.js, optional ffmpeg, and install dependencies. + +inputs: + node-version: + description: Node.js version to use. + required: true + arch: + description: Target architecture for dependency installation. + required: true + install-ffmpeg: + description: Whether to install ffmpeg on Windows. + required: false + default: 'false' + +runs: + using: composite + steps: + - uses: actions/setup-python@v5 + with: + python-version: '3.11' + + - name: Use Node.js ${{ inputs.node-version }} + uses: actions/setup-node@v4 + with: + node-version: ${{ inputs.node-version }} + cache: yarn + + - name: Install ffmpeg + if: ${{ runner.os == 'Windows' && inputs.install-ffmpeg == 'true' }} + shell: bash + run: choco install ffmpeg --yes --no-progress + + - name: Install and build dependencies + shell: bash + run: yarn + env: + npm_config_arch: ${{ inputs.arch }} + TARGET_ARCH: ${{ inputs.arch }} diff --git a/.github/actions/setup-windows-signing/action.yml b/.github/actions/setup-windows-signing/action.yml new file mode 100644 index 00000000000..7b09408aa39 --- /dev/null +++ b/.github/actions/setup-windows-signing/action.yml @@ -0,0 +1,35 @@ +name: Setup Windows Signing +description: Install Azure Code Signing prerequisites and authenticate. + +inputs: + enabled: + description: Whether Windows signing setup should run. + required: false + default: 'false' + azure-client-id: + description: Azure Code Signing client ID. + required: false + azure-tenant-id: + description: Azure Code Signing tenant ID. + required: false + +runs: + using: composite + steps: + - name: Install Azure Code Signing Client + if: ${{ runner.os == 'Windows' && inputs.enabled == 'true' }} + shell: pwsh + run: | + $acsZip = Join-Path $env:RUNNER_TEMP "acs.zip" + $acsDir = Join-Path $env:RUNNER_TEMP "acs" + Invoke-WebRequest -Uri https://www.nuget.org/api/v2/package/Microsoft.Trusted.Signing.Client/1.0.95 -OutFile $acsZip -Verbose + Expand-Archive $acsZip -Destination $acsDir -Force -Verbose + Copy-Item -Path "C:\Program Files (x86)\Windows Kits\10\bin\10.0.22621.0\x64\*" -Include signtool.exe,signtool.exe.manifest,Microsoft.Windows.Build.Signing.mssign32.dll.manifest,mssign32.dll,Microsoft.Windows.Build.Signing.wintrust.dll.manifest,wintrust.dll,Microsoft.Windows.Build.Appx.AppxSip.dll.manifest,AppxSip.dll,Microsoft.Windows.Build.Appx.AppxPackaging.dll.manifest,AppxPackaging.dll,Microsoft.Windows.Build.Appx.OpcServices.dll.manifest,OpcServices.dll -Destination "node_modules\electron-winstaller\vendor" -Verbose + + - name: Azure Login (OIDC) + if: ${{ runner.os == 'Windows' && inputs.enabled == 'true' }} + uses: azure/login@v2 + with: + client-id: ${{ inputs.azure-client-id }} + tenant-id: ${{ inputs.azure-tenant-id }} + allow-no-subscriptions: true diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ca20e730b7b..42420c2e36d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -89,19 +89,10 @@ jobs: repository: ${{ inputs.repository || github.repository }} ref: ${{ inputs.ref }} submodules: recursive - - uses: actions/setup-python@v5 - with: - python-version: '3.11' - - name: Use Node.js ${{ env.NODE_VERSION }} - uses: actions/setup-node@v4 + - uses: ./.github/actions/setup-ci-environment with: node-version: ${{ env.NODE_VERSION }} - cache: yarn - - name: Install and build dependencies - run: yarn - env: - npm_config_arch: ${{ matrix.arch }} - TARGET_ARCH: ${{ matrix.arch }} + arch: ${{ matrix.arch }} - name: Validate macOS version if: runner.os == 'macOS' run: yarn validate-macos-version @@ -133,22 +124,11 @@ jobs: run: yarn test:unit - name: Run script tests run: yarn test:script - - name: Install Azure Code Signing Client - if: ${{ runner.os == 'Windows' && inputs.sign }} - run: | - $acsZip = Join-Path $env:RUNNER_TEMP "acs.zip" - $acsDir = Join-Path $env:RUNNER_TEMP "acs" - Invoke-WebRequest -Uri https://www.nuget.org/api/v2/package/Microsoft.Trusted.Signing.Client/1.0.95 -OutFile $acsZip -Verbose - Expand-Archive $acsZip -Destination $acsDir -Force -Verbose - # Replace ancient signtool in electron-winstall with one that supports ACS - Copy-Item -Path "C:\Program Files (x86)\Windows Kits\10\bin\10.0.22621.0\x64\*" -Include signtool.exe,signtool.exe.manifest,Microsoft.Windows.Build.Signing.mssign32.dll.manifest,mssign32.dll,Microsoft.Windows.Build.Signing.wintrust.dll.manifest,wintrust.dll,Microsoft.Windows.Build.Appx.AppxSip.dll.manifest,AppxSip.dll,Microsoft.Windows.Build.Appx.AppxPackaging.dll.manifest,AppxPackaging.dll,Microsoft.Windows.Build.Appx.OpcServices.dll.manifest,OpcServices.dll -Destination "node_modules\electron-winstaller\vendor" -Verbose - - name: Azure Login (OIDC) - if: ${{ runner.os == 'Windows' && inputs.sign }} - uses: azure/login@v2 + - uses: ./.github/actions/setup-windows-signing with: - client-id: ${{ secrets.AZURE_CODE_SIGNING_CLIENT_ID }} - tenant-id: ${{ secrets.AZURE_CODE_SIGNING_TENANT_ID }} - allow-no-subscriptions: true + enabled: ${{ inputs.sign }} + azure-client-id: ${{ secrets.AZURE_CODE_SIGNING_CLIENT_ID }} + azure-tenant-id: ${{ secrets.AZURE_CODE_SIGNING_TENANT_ID }} - name: Package production app run: yarn package env: @@ -192,22 +172,11 @@ jobs: repository: ${{ inputs.repository || github.repository }} ref: ${{ inputs.ref }} submodules: recursive - - uses: actions/setup-python@v5 - with: - python-version: '3.11' - - name: Use Node.js ${{ env.NODE_VERSION }} - uses: actions/setup-node@v4 + - uses: ./.github/actions/setup-ci-environment with: node-version: ${{ env.NODE_VERSION }} - cache: yarn - - name: Install ffmpeg - if: runner.os == 'Windows' - run: choco install ffmpeg --yes --no-progress - - name: Install and build dependencies - run: yarn - env: - npm_config_arch: ${{ matrix.arch }} - TARGET_ARCH: ${{ matrix.arch }} + arch: ${{ matrix.arch }} + install-ffmpeg: 'true' - name: Build production app run: yarn build:prod env: @@ -222,21 +191,11 @@ jobs: KEY_PASSWORD: ${{ secrets.APPLE_APPLICATION_CERT_PASSWORD }} npm_config_arch: ${{ matrix.arch }} TARGET_ARCH: ${{ matrix.arch }} - - name: Install Azure Code Signing Client - if: ${{ runner.os == 'Windows' && inputs.sign }} - run: | - $acsZip = Join-Path $env:RUNNER_TEMP "acs.zip" - $acsDir = Join-Path $env:RUNNER_TEMP "acs" - Invoke-WebRequest -Uri https://www.nuget.org/api/v2/package/Microsoft.Trusted.Signing.Client/1.0.95 -OutFile $acsZip -Verbose - Expand-Archive $acsZip -Destination $acsDir -Force -Verbose - Copy-Item -Path "C:\Program Files (x86)\Windows Kits\10\bin\10.0.22621.0\x64\*" -Include signtool.exe,signtool.exe.manifest,Microsoft.Windows.Build.Signing.mssign32.dll.manifest,mssign32.dll,Microsoft.Windows.Build.Signing.wintrust.dll.manifest,wintrust.dll,Microsoft.Windows.Build.Appx.AppxSip.dll.manifest,AppxSip.dll,Microsoft.Windows.Build.Appx.AppxPackaging.dll.manifest,AppxPackaging.dll,Microsoft.Windows.Build.Appx.OpcServices.dll.manifest,OpcServices.dll -Destination "node_modules\electron-winstaller\vendor" -Verbose - - name: Azure Login (OIDC) - if: ${{ runner.os == 'Windows' && inputs.sign }} - uses: azure/login@v2 + - uses: ./.github/actions/setup-windows-signing with: - client-id: ${{ secrets.AZURE_CODE_SIGNING_CLIENT_ID }} - tenant-id: ${{ secrets.AZURE_CODE_SIGNING_TENANT_ID }} - allow-no-subscriptions: true + enabled: ${{ inputs.sign }} + azure-client-id: ${{ secrets.AZURE_CODE_SIGNING_CLIENT_ID }} + azure-tenant-id: ${{ secrets.AZURE_CODE_SIGNING_TENANT_ID }} - name: Package production app run: yarn package env: From 7dfa5c4ca02d63a9fc1599a83b10e19038c9caa8 Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Thu, 19 Mar 2026 14:08:17 +0100 Subject: [PATCH 10/26] Add E2E smoke tests docs; remove Playwright script Add comprehensive documentation for the new Playwright-based E2E smoke test harness at docs/technical/e2e-smoke-tests.md, describing coverage, launch modes, CI integration, updater mocking, artifacts, and usage. Remove the deprecated script/playwright-electron.ts helper which is no longer used by the test harness. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/technical/e2e-smoke-tests.md | 239 ++++++++++++++++++++++++++++++ script/playwright-electron.ts | 110 -------------- 2 files changed, 239 insertions(+), 110 deletions(-) create mode 100644 docs/technical/e2e-smoke-tests.md delete mode 100644 script/playwright-electron.ts diff --git a/docs/technical/e2e-smoke-tests.md b/docs/technical/e2e-smoke-tests.md new file mode 100644 index 00000000000..d22c4ab500d --- /dev/null +++ b/docs/technical/e2e-smoke-tests.md @@ -0,0 +1,239 @@ +# E2E Smoke Tests + +This document explains the end-to-end smoke test harness added for GitHub +Desktop, what it covers, how it runs locally and in CI, and which repository +files make up the implementation. + +## Overview + +The smoke suite uses Playwright's Electron support to launch the real Desktop +application and drive a small set of critical user paths. The focus is not broad +UI coverage. The suite is intended to protect the app's highest-risk integration +boundaries: + +- app startup and first-run flow +- opening an existing repository +- committing and switching branches +- updater state transitions and platform-specific update behavior + +The suite is deliberately small and runs serially in one Electron session. That +keeps it practical for CI and useful for iterative development. + +## What The Suite Covers + +The current smoke spec lives in `app/test/e2e/app-launch.e2e.ts` and covers: + +- launching Desktop and completing the welcome flow +- opening the smoke repository from disk +- verifying the changed file appears in the diff +- creating a commit +- creating a branch and switching back +- verifying startup update checks against the mock update server +- verifying About dialog update states +- verifying update-available and quit-during-download behavior + +These tests intentionally mix UI assertions with repository-level verification by +checking Git state directly in the smoke repository. + +## File Layout + +The main files added or changed for the E2E harness are: + +- `app/test/e2e/app-launch.e2e.ts` + - the smoke suite itself +- `app/test/e2e/e2e-fixtures.ts` + - Playwright/Electron fixtures, launch mode selection, tracing, video output, + and Windows updater cleanup +- `app/test/e2e/mock-update-server.ts` + - an in-process HTTP server used to simulate updater responses for macOS and + Windows +- `app/test/e2e/test-helpers.ts` + - helpers for creating and validating the smoke test repository +- `app/test/e2e/playwright.config.ts` + - Playwright configuration for the smoke suite + +## How The App Is Launched + +The fixture supports three effective launch modes. + +### Installed app + +If `DESKTOP_E2E_APP_PATH` is set, the fixture launches the executable at that +path. This is the mode used by CI after the app has been packaged and installed. + +This matters most on Windows, where Squirrel update behavior depends on running +from a real installed application layout. + +### Packaged app + +If `DESKTOP_E2E_APP_PATH` is not set and `DESKTOP_E2E_APP_MODE` is not set to +`unpackaged`, the fixture resolves the packaged app path from `dist` and launches +that executable directly. + +This is the default E2E mode because CI uses production-like packaged artifacts. + +### Unpackaged app + +If `DESKTOP_E2E_APP_MODE=unpackaged`, the fixture launches `out/main.js` +instead of a packaged executable. + +This mode exists for local iteration. It avoids the need to fully package and +sign the app just to run the smoke suite while still using a production webpack +bundle and staged resources. + +## Local Commands + +The branch adds two ways to run the suite locally. + +### Packaged mode + +```bash +yarn test:e2e:packaged +``` + +This builds a packaged production app and then runs the E2E suite. + +### Unpackaged mode + +```bash +yarn test:e2e:unpackaged +``` + +This builds a production-configured staged app in `out/` and runs the same E2E +suite against `out/main.js`. + +The unpackaged build path uses `DESKTOP_SKIP_PACKAGE=1` so `script/build.ts` +stages the app without invoking the final packaging step. + +### Default alias + +```bash +yarn test:e2e +``` + +This remains the packaged path and is equivalent to `yarn test:e2e:packaged`. + +## Smoke Repository Setup + +The suite uses a throwaway local Git repository created in the system temp +directory. + +`app/test/e2e/test-helpers.ts` is responsible for: + +- creating the repository +- initializing Git +- configuring a local author name and email +- creating an initial commit +- adding an uncommitted smoke file used by the tests + +The tests then verify Desktop's behavior both through the UI and by checking the +repository state directly with Git commands. + +## Updater Testing + +Updater behavior is tested through a local HTTP server defined in +`app/test/e2e/mock-update-server.ts`. + +### Build-time updater URL override + +`app/app-info.ts` now uses `DESKTOP_E2E_UPDATES_URL` when present. This is what +lets E2E builds point the app at the local mock update server instead of the +real update service. + +### macOS behavior + +For macOS, the mock server serves a Squirrel.Mac-style JSON response. + +- `no-update` returns HTTP 204 +- `update-available` returns a JSON payload with a fake zip URL + +The fake download stays open rather than completing, which keeps the app in the +"Downloading update…" state without requiring a valid signed update archive. + +### Windows behavior + +For Windows, the mock server serves Squirrel.Windows-style responses. + +- requests for `RELEASES` receive a text manifest +- requests for `.nupkg` receive a fake long-lived binary response + +This is necessary because Windows updater behavior does not use the macOS JSON +contract. + +### Mock server control plane + +The mock server exposes a simple control surface under `/_control/`. + +The tests use it to: + +- switch between `no-update` and `update-available` +- reset the captured request log +- inspect which requests the app made + +## CI Design + +The `e2e-smoke` job in `.github/workflows/ci.yml` runs separately from the main +`build` job. + +It currently: + +- checks out the repository +- uses the shared CI environment setup action +- builds the production app with `DESKTOP_E2E_UPDATES_URL` pointed at the mock + server +- uses the shared Windows signing action when needed +- packages the app +- installs the app on macOS and Windows +- exports `DESKTOP_E2E_APP_PATH` +- runs `yarn test:e2e:run:packaged` +- uploads `playwright-videos` as artifacts so traces and videos are retained + +The CI job intentionally tests production-like packaged or installed artifacts. +This catches failures that do not show up when running only webpack output. + +## Windows-Specific Notes + +Windows needed a few extra pieces to keep the suite stable. + +- the smoke suite only runs updater coverage against an installed app path on + Windows +- the workflow installs the generated Squirrel setup executable silently and + discovers the installed `GitHubDesktop.exe` path +- installer failures dump Squirrel log files for diagnosis +- the E2E fixtures kill lingering `Update.exe` and `GitHubDesktop.exe` process + trees during teardown to avoid hangs and races with the mock server + +## Videos, Traces, and Diagnostics + +The suite writes output to `playwright-videos`. + +Artifacts include: + +- Playwright videos recorded through the Electron fixture +- trace zip files saved from the Playwright context + +The workflow uploads that directory as an artifact so CI failures can be +inspected after the run completes. + +The fixture also forwards renderer console errors and page errors to the job log +to make failures easier to diagnose when the app dies before Playwright can make +useful assertions. + +## Limitations And Tradeoffs + +- The suite is intentionally narrow. It protects critical integration paths, not + the entire UI surface. +- Updater tests simulate update availability rather than downloading valid + signed release artifacts. +- Windows updater behavior is realistic only when running from an installed app, + which is why CI uses installer-based setup there. +- Local unpackaged mode is meant for fast iteration, not as a perfect substitute + for packaged/install-time validation. + +## When To Use Which Mode + +- Use `yarn test:e2e:unpackaged` when iterating locally on the smoke suite or + nearby app behavior. +- Use `yarn test:e2e:packaged` when you need parity with the packaged runtime. +- Rely on the CI `e2e-smoke` job for the final check against production-like + packaged and installed artifacts. diff --git a/script/playwright-electron.ts b/script/playwright-electron.ts deleted file mode 100644 index 265e582990b..00000000000 --- a/script/playwright-electron.ts +++ /dev/null @@ -1,110 +0,0 @@ -/** - * Launch GitHub Desktop via Playwright for agent-driven UI verification. - * - * This script starts the app and keeps it running so that an agent can - * interact with it using Playwright browser automation tools (click - * elements, take screenshots, inspect the DOM, etc.). - * - * Set RECORD_VIDEO=1 to enable video recording of the session. The video - * is saved to the project root when the app closes. - * - * Usage: - * npx ts-node -P script/tsconfig.json script/playwright-electron.ts - * RECORD_VIDEO=1 npx ts-node -P script/tsconfig.json script/playwright-electron.ts - * - * Prerequisites: - * yarn build:dev # or yarn compile:dev — need out/main.js - */ - -/* eslint-disable no-sync */ - -import path from 'path' -import fs from 'fs' -import os from 'os' -import { _electron as electron } from 'playwright' - -const projectRoot = path.resolve(__dirname, '..') -const appEntryPoint = path.join(projectRoot, 'out', 'main.js') -const userDataDir = path.join(os.tmpdir(), 'github-desktop-playwright-verify') -const videosDir = path.join(projectRoot, 'playwright-videos') - -if (!fs.existsSync(appEntryPoint)) { - console.error( - `Error: Built app not found at ${appEntryPoint}\nRun 'yarn build:dev' or 'yarn compile:dev' first.` - ) - process.exit(1) -} - -fs.rmSync(userDataDir, { recursive: true, force: true }) -fs.mkdirSync(userDataDir, { recursive: true }) - -const recordVideo = process.env.RECORD_VIDEO === '1' -const repoPath = process.argv[2] // Optional: path to a repo to open - -async function main() { - if (recordVideo) { - fs.mkdirSync(videosDir, { recursive: true }) - console.log( - `Video recording enabled — videos will be saved to ${videosDir}` - ) - } - - const appArgs = [appEntryPoint, `--user-data-dir=${userDataDir}`] - if (repoPath) { - appArgs.push(`--cli-open=${repoPath}`) - console.log(`Opening repository: ${repoPath}`) - } - - console.log('Launching GitHub Desktop via Playwright…') - const app = await electron.launch({ - args: appArgs, - env: { - ...process.env, - // Isolate all user config so the agent doesn't read or modify the - // developer's real settings. - GIT_CONFIG_GLOBAL: path.join(userDataDir, '.gitconfig'), - GIT_CONFIG_SYSTEM: path.join(userDataDir, '.gitconfig-system'), - XDG_CONFIG_HOME: path.join(userDataDir, '.config'), - // Prevent SSH from using the developer's real keys - SSH_AUTH_SOCK: '', - GIT_SSH_COMMAND: 'false', - }, - recordVideo: recordVideo - ? { dir: videosDir, size: { width: 1280, height: 800 } } - : undefined, - timeout: 30000, - }) - - const window = await app.firstWindow() - console.log(`Window title: ${await window.title()}`) - console.log(`Window URL: ${window.url()}`) - console.log('\nPlaywright Electron session is active.') - console.log( - 'Use Playwright browser automation tools to interact with the app.' - ) - console.log('Press Ctrl+C to close.\n') - - // When the process is interrupted, close gracefully so videos are flushed - process.on('SIGINT', async () => { - console.log('\nClosing app…') - await app.close() - - if (recordVideo) { - const video = window.video() - if (video) { - const videoPath = await video.path() - console.log(`Video saved: ${videoPath}`) - } - } - - process.exit(0) - }) - - // Keep process alive until interrupted - await new Promise(() => {}) -} - -main().catch(err => { - console.error(err) - process.exit(1) -}) From f3b3ff0149a4b092eb5ce039c931a3512495d454 Mon Sep 17 00:00:00 2001 From: tidy-dev <75402236+tidy-dev@users.noreply.github.com> Date: Thu, 19 Mar 2026 09:56:38 -0400 Subject: [PATCH 11/26] Extract shared branch name rule validation Move checkBranchNameRules() and renderBranchNameRuleError() into a shared utility at ui/lib/branch-name-rule-validation.tsx. Both the create branch and rename branch dialogs now use these shared functions, eliminating ~80 lines of duplicated logic. The shared module also exports IBranchRuleError as a reusable type for the error state shape both dialogs use. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../ui/create-branch/create-branch-dialog.tsx | 154 +++--------------- .../ui/lib/branch-name-rule-validation.tsx | 145 +++++++++++++++++ .../ui/rename-branch/rename-branch-dialog.tsx | 152 +++-------------- 3 files changed, 185 insertions(+), 266 deletions(-) create mode 100644 app/src/ui/lib/branch-name-rule-validation.tsx diff --git a/app/src/ui/create-branch/create-branch-dialog.tsx b/app/src/ui/create-branch/create-branch-dialog.tsx index f8d49dab2a3..20e70fe7548 100644 --- a/app/src/ui/create-branch/create-branch-dialog.tsx +++ b/app/src/ui/create-branch/create-branch-dialog.tsx @@ -1,9 +1,6 @@ import * as React from 'react' -import { - Repository, - isRepositoryWithGitHubRepository, -} from '../../models/repository' +import { Repository } from '../../models/repository' import { Dispatcher } from '../dispatcher' import { Branch, StartPoint } from '../../models/branch' import { Row } from '../lib/row' @@ -31,12 +28,13 @@ import { CommitOneLine } from '../../models/commit' import { PopupType } from '../../models/popup' import { RepositorySettingsTab } from '../repository-settings/repository-settings' import { isRepositoryWithForkedGitHubRepository } from '../../models/repository' -import { API, APIRepoRuleType, IAPIRepoRuleset } from '../../lib/api' +import { IAPIRepoRuleset } from '../../lib/api' import { Account } from '../../models/account' -import { getAccountForRepository } from '../../lib/get-account-for-repository' -import { InputError } from '../lib/input-description/input-error' -import { InputWarning } from '../lib/input-description/input-warning' -import { parseRepoRules, useRepoRulesLogic } from '../../lib/helpers/repo-rules' +import { + IBranchRuleError, + checkBranchNameRules, + renderBranchNameRuleError, +} from '../lib/branch-name-rule-validation' interface ICreateBranchProps { readonly repository: Repository @@ -74,7 +72,7 @@ interface ICreateBranchProps { } interface ICreateBranchState { - readonly currentError: { error: Error; isWarning: boolean } | null + readonly currentError: IBranchRuleError | null readonly branchName: string readonly startPoint: StartPoint @@ -215,37 +213,6 @@ export class CreateBranch extends React.Component< } } - private renderBranchNameErrors() { - const { currentError } = this.state - if (!currentError) { - return null - } - - if (currentError.isWarning) { - return ( - - - {currentError.error.message} - - - ) - } else { - return ( - - - {currentError.error.message} - - - ) - } - } - private onBaseBranchChanged = (startPoint: StartPoint) => { this.setState({ startPoint, @@ -276,7 +243,11 @@ export class CreateBranch extends React.Component< onValueChange={this.onBranchNameChange} /> - {this.renderBranchNameErrors()} + {renderBranchNameRuleError( + this.state.currentError, + this.ERRORS_ID, + this.state.branchName + )} {renderBranchNameExistsOnRemoteWarning( this.state.branchName, @@ -347,114 +318,29 @@ export class CreateBranch extends React.Component< }) } - /** - * Checks repo rules to see if the provided branch name is valid for the - * current user and repository. The "get all rules for a branch" endpoint - * is called first, and if a "creation" or "branch name" rule is found, - * then those rulesets are checked to see if the current user can bypass - * them. - */ private checkBranchRules = async (branchName: string) => { if ( this.state.branchName !== branchName || - this.props.accounts.length === 0 || - !isRepositoryWithGitHubRepository(this.props.repository) || branchName === '' || this.state.currentError !== null ) { return } - const account = getAccountForRepository( + const result = await checkBranchNameRules( + branchName, this.props.accounts, - this.props.repository - ) - - if ( - account === null || - !useRepoRulesLogic(account, this.props.repository) - ) { - return - } - - const api = API.fromAccount(account) - const branchRules = await api.fetchRepoRulesForBranch( - this.props.repository.gitHubRepository.owner.login, - this.props.repository.gitHubRepository.name, - branchName - ) - - // Make sure user branch name hasn't changed during api call - if (this.state.branchName !== branchName) { - return - } - - // filter the rules to only the relevant ones and get their IDs. use a Set to dedupe. - const toCheck = new Set( - branchRules - .filter( - r => - r.type === APIRepoRuleType.Creation || - r.type === APIRepoRuleType.BranchNamePattern - ) - .map(r => r.ruleset_id) + this.props.repository, + this.props.cachedRepoRulesets ) - // there are no relevant rules for this branch name, so return - if (toCheck.size === 0) { - return - } - - // check for actual failures - const { branchNamePatterns, creationRestricted } = await parseRepoRules( - branchRules, - this.props.cachedRepoRulesets, - this.props.repository - ) - - // Make sure user branch name hasn't changed during parsing of repo rules - // (async due to a config retrieval of users with commit signing repo rules) + // Make sure user branch name hasn't changed during async calls if (this.state.branchName !== branchName) { return } - const { status } = branchNamePatterns.getFailedRules(branchName) - - // Only possible kind of failures is branch name pattern failures and creation restriction - if (creationRestricted !== true && status === 'pass') { - return - } - - // check cached rulesets to see which ones the user can bypass - let cannotBypass = false - for (const id of toCheck) { - const rs = this.props.cachedRepoRulesets.get(id) - - if (rs?.current_user_can_bypass !== 'always') { - // the user cannot bypass, so stop checking - cannotBypass = true - break - } - } - - if (cannotBypass) { - this.setState({ - currentError: { - error: new Error( - `Branch name '${branchName}' is restricted by repo rules.` - ), - isWarning: false, - }, - }) - } else { - this.setState({ - currentError: { - error: new Error( - `Branch name '${branchName}' is restricted by repo rules, but you can bypass them. Proceed with caution!` - ), - isWarning: true, - }, - }) + if (result !== null) { + this.setState({ currentError: result }) } } diff --git a/app/src/ui/lib/branch-name-rule-validation.tsx b/app/src/ui/lib/branch-name-rule-validation.tsx new file mode 100644 index 00000000000..e3bd4167180 --- /dev/null +++ b/app/src/ui/lib/branch-name-rule-validation.tsx @@ -0,0 +1,145 @@ +import * as React from 'react' + +import { + Repository, + isRepositoryWithGitHubRepository, +} from '../../models/repository' +import { API, APIRepoRuleType, IAPIRepoRuleset } from '../../lib/api' +import { Account } from '../../models/account' +import { getAccountForRepository } from '../../lib/get-account-for-repository' +import { parseRepoRules, useRepoRulesLogic } from '../../lib/helpers/repo-rules' +import { InputError } from './input-description/input-error' +import { InputWarning } from './input-description/input-warning' +import { Row } from './row' + +/** The result of a branch name rule check. */ +export interface IBranchRuleError { + readonly error: Error + readonly isWarning: boolean +} + +/** + * Checks repo rules to see if the provided branch name is valid for the + * current user and repository. The "get all rules for a branch" endpoint + * is called first, and if a "creation" or "branch name" rule is found, + * then those rulesets are checked to see if the current user can bypass + * them. + * + * Returns `null` if the branch name passes all rules or if validation + * cannot be performed (e.g. no accounts, non-GitHub repo). + */ +export async function checkBranchNameRules( + branchName: string, + accounts: ReadonlyArray, + repository: Repository, + cachedRepoRulesets: ReadonlyMap +): Promise { + if ( + accounts.length === 0 || + !isRepositoryWithGitHubRepository(repository) || + branchName === '' + ) { + return null + } + + const account = getAccountForRepository(accounts, repository) + + if (account === null || !useRepoRulesLogic(account, repository)) { + return null + } + + const api = API.fromAccount(account) + const branchRules = await api.fetchRepoRulesForBranch( + repository.gitHubRepository.owner.login, + repository.gitHubRepository.name, + branchName + ) + + // filter the rules to only the relevant ones and get their IDs. use a Set to dedupe. + const toCheck = new Set( + branchRules + .filter( + r => + r.type === APIRepoRuleType.Creation || + r.type === APIRepoRuleType.BranchNamePattern + ) + .map(r => r.ruleset_id) + ) + + // there are no relevant rules for this branch name + if (toCheck.size === 0) { + return null + } + + // check for actual failures + const { branchNamePatterns, creationRestricted } = await parseRepoRules( + branchRules, + cachedRepoRulesets, + repository + ) + + const { status } = branchNamePatterns.getFailedRules(branchName) + + if (creationRestricted !== true && status === 'pass') { + return null + } + + // check cached rulesets to see which ones the user can bypass + let cannotBypass = false + for (const id of toCheck) { + const rs = cachedRepoRulesets.get(id) + + if (rs?.current_user_can_bypass !== 'always') { + cannotBypass = true + break + } + } + + if (cannotBypass) { + return { + error: new Error( + `Branch name '${branchName}' is restricted by repo rules.` + ), + isWarning: false, + } + } + + return { + error: new Error( + `Branch name '${branchName}' is restricted by repo rules, but you can bypass them. Proceed with caution!` + ), + isWarning: true, + } +} + +/** + * Renders an error or warning row for branch name rule violations. + * Returns `null` if there is no error. + */ +export function renderBranchNameRuleError( + currentError: IBranchRuleError | null, + errorsId: string, + trackedUserInput: string +): React.ReactElement | null { + if (currentError === null) { + return null + } + + if (currentError.isWarning) { + return ( + + + {currentError.error.message} + + + ) + } + + return ( + + + {currentError.error.message} + + + ) +} diff --git a/app/src/ui/rename-branch/rename-branch-dialog.tsx b/app/src/ui/rename-branch/rename-branch-dialog.tsx index 66bc32907ae..dd404e7e6c5 100644 --- a/app/src/ui/rename-branch/rename-branch-dialog.tsx +++ b/app/src/ui/rename-branch/rename-branch-dialog.tsx @@ -1,22 +1,19 @@ import * as React from 'react' import { Dispatcher } from '../dispatcher' -import { - Repository, - isRepositoryWithGitHubRepository, -} from '../../models/repository' +import { Repository } from '../../models/repository' import { Branch } from '../../models/branch' import { Dialog, DialogContent, DialogFooter } from '../dialog' import { renderBranchHasRemoteWarning } from '../lib/branch-name-warnings' import { OkCancelButtonGroup } from '../dialog/ok-cancel-button-group' import { RefNameTextBox } from '../lib/ref-name-text-box' -import { API, APIRepoRuleType, IAPIRepoRuleset } from '../../lib/api' +import { IAPIRepoRuleset } from '../../lib/api' import { Account } from '../../models/account' -import { getAccountForRepository } from '../../lib/get-account-for-repository' -import { InputError } from '../lib/input-description/input-error' -import { InputWarning } from '../lib/input-description/input-warning' -import { parseRepoRules, useRepoRulesLogic } from '../../lib/helpers/repo-rules' -import { Row } from '../lib/row' +import { + IBranchRuleError, + checkBranchNameRules, + renderBranchNameRuleError, +} from '../lib/branch-name-rule-validation' interface IRenameBranchProps { readonly dispatcher: Dispatcher @@ -29,7 +26,7 @@ interface IRenameBranchProps { interface IRenameBranchState { readonly newName: string - readonly currentError: { error: Error; isWarning: boolean } | null + readonly currentError: IBranchRuleError | null } export class RenameBranch extends React.Component< @@ -83,7 +80,11 @@ export class RenameBranch extends React.Component< onValueChange={this.onNameChange} /> - {this.renderBranchNameErrors()} + {renderBranchNameRuleError( + this.state.currentError, + this.ERRORS_ID, + this.state.newName + )} @@ -96,34 +97,6 @@ export class RenameBranch extends React.Component< ) } - private renderBranchNameErrors() { - const { currentError } = this.state - if (!currentError) { - return null - } - - if (currentError.isWarning) { - return ( - - - {currentError.error.message} - - - ) - } else { - return ( - - - {currentError.error.message} - - - ) - } - } - private onNameChange = (name: string) => { this.setState({ newName: name, currentError: null }) @@ -140,114 +113,29 @@ export class RenameBranch extends React.Component< } } - /** - * Checks repo rules to see if the provided branch name is valid for the - * current user and repository. The "get all rules for a branch" endpoint - * is called first, and if a "creation" or "branch name" rule is found, - * then those rulesets are checked to see if the current user can bypass - * them. - */ private checkBranchRules = async (branchName: string) => { if ( this.state.newName !== branchName || - this.props.accounts.length === 0 || - !isRepositoryWithGitHubRepository(this.props.repository) || branchName === '' || this.state.currentError !== null ) { return } - const account = getAccountForRepository( + const result = await checkBranchNameRules( + branchName, this.props.accounts, - this.props.repository - ) - - if ( - account === null || - !useRepoRulesLogic(account, this.props.repository) - ) { - return - } - - const api = API.fromAccount(account) - const branchRules = await api.fetchRepoRulesForBranch( - this.props.repository.gitHubRepository.owner.login, - this.props.repository.gitHubRepository.name, - branchName - ) - - // Make sure user branch name hasn't changed during api call - if (this.state.newName !== branchName) { - return - } - - // filter the rules to only the relevant ones and get their IDs. use a Set to dedupe. - const toCheck = new Set( - branchRules - .filter( - r => - r.type === APIRepoRuleType.Creation || - r.type === APIRepoRuleType.BranchNamePattern - ) - .map(r => r.ruleset_id) - ) - - // there are no relevant rules for this branch name, so return - if (toCheck.size === 0) { - return - } - - // check for actual failures - const { branchNamePatterns, creationRestricted } = await parseRepoRules( - branchRules, - this.props.cachedRepoRulesets, - this.props.repository + this.props.repository, + this.props.cachedRepoRulesets ) - // Make sure user branch name hasn't changed during parsing of repo rules - // (async due to a config retrieval of users with commit signing repo rules) + // Make sure user branch name hasn't changed during async calls if (this.state.newName !== branchName) { return } - const { status } = branchNamePatterns.getFailedRules(branchName) - - // Only possible kind of failures is branch name pattern failures and creation restriction - if (creationRestricted !== true && status === 'pass') { - return - } - - // check cached rulesets to see which ones the user can bypass - let cannotBypass = false - for (const id of toCheck) { - const rs = this.props.cachedRepoRulesets.get(id) - - if (rs?.current_user_can_bypass !== 'always') { - // the user cannot bypass, so stop checking - cannotBypass = true - break - } - } - - if (cannotBypass) { - this.setState({ - currentError: { - error: new Error( - `Branch name '${branchName}' is restricted by repo rules.` - ), - isWarning: false, - }, - }) - } else { - this.setState({ - currentError: { - error: new Error( - `Branch name '${branchName}' is restricted by repo rules, but you can bypass them. Proceed with caution!` - ), - isWarning: true, - }, - }) + if (result !== null) { + this.setState({ currentError: result }) } } From 7e8219d9f61a948189c349a154fd05793ed7def6 Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Thu, 19 Mar 2026 17:36:00 +0100 Subject: [PATCH 12/26] Update globals.mts --- app/test/globals.mts | 55 +------------------------------------------- 1 file changed, 1 insertion(+), 54 deletions(-) diff --git a/app/test/globals.mts b/app/test/globals.mts index d57bee04fd1..b33fab090c4 100644 --- a/app/test/globals.mts +++ b/app/test/globals.mts @@ -40,62 +40,9 @@ Object.assign(globalThis, { BroadcastChannel: undefined, }) -Object.assign(globalThis, { - Event: window.Event, - CustomEvent: window.CustomEvent, -}) - -type DialogElement = HTMLElement & { - open?: boolean - showModal?: () => void - close?: () => void -} - -const dialogPrototype = HTMLElement.prototype as DialogElement - -if (typeof dialogPrototype.showModal !== 'function') { - dialogPrototype.showModal = function () { - this.open = true - this.setAttribute('open', '') - } -} - -if (typeof dialogPrototype.close !== 'function') { - dialogPrototype.close = function () { - this.open = false - this.removeAttribute('open') - } -} - -if (globalThis.ResizeObserver === undefined) { - class TestResizeObserver { - public observe() {} - public disconnect() {} - } - - Object.assign(globalThis, { ResizeObserver: TestResizeObserver }) -} - -Object.defineProperty(HTMLFormElement.prototype, 'requestSubmit', { - configurable: true, - writable: true, - value: function () { - this.dispatchEvent( - new window.Event('submit', { bubbles: true, cancelable: true }) - ) - }, -}) - mock.module('electron', { namedExports: { shell: {}, - ipcRenderer: { - on: mock.fn(() => {}), - once: mock.fn(() => {}), - send: mock.fn(() => {}), - sendSync: mock.fn(() => {}), - invoke: mock.fn(async () => undefined), - removeListener: mock.fn(() => {}), - }, + ipcRenderer: { on: mock.fn(x => {}) }, }, }) From f08f0c5dfc16f14a3d53221940e67e2eb986dd31 Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Thu, 19 Mar 2026 17:41:42 +0100 Subject: [PATCH 13/26] Cleanup --- app/test/e2e/e2e-fixtures.ts | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/app/test/e2e/e2e-fixtures.ts b/app/test/e2e/e2e-fixtures.ts index 147d0398baa..f21f9bb71af 100644 --- a/app/test/e2e/e2e-fixtures.ts +++ b/app/test/e2e/e2e-fixtures.ts @@ -95,7 +95,7 @@ function getE2ELaunchOptions() { } } -function killLingeringWindowsUpdaterProcesses() { +export function terminateWindowsUpdaterProcesses() { if (process.platform !== 'win32') { return } @@ -108,10 +108,6 @@ function killLingeringWindowsUpdaterProcesses() { } } -export function terminateWindowsUpdaterProcesses() { - killLingeringWindowsUpdaterProcesses() -} - // ── Helpers exposed to tests ──────────────────────────────────────── export function controlMockServer(action: string): Promise { @@ -190,12 +186,9 @@ export const test = base.extend<{}, E2EFixtures>({ await use(app) - if (process.platform === 'win32') { - killLingeringWindowsUpdaterProcesses() - } - + terminateWindowsUpdaterProcesses() await app.close().catch(() => {}) - killLingeringWindowsUpdaterProcesses() + terminateWindowsUpdaterProcesses() await new Promise(resolve => setTimeout(resolve, 1000)) }, { scope: 'worker' }, From d3ebe217c55882d4f9e696fda0e782cb13f81062 Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Thu, 19 Mar 2026 17:44:02 +0100 Subject: [PATCH 14/26] Improve types --- app/test/e2e/e2e-fixtures.ts | 9 ++++++++- app/test/e2e/mock-update-server.ts | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/app/test/e2e/e2e-fixtures.ts b/app/test/e2e/e2e-fixtures.ts index f21f9bb71af..3d8289d517f 100644 --- a/app/test/e2e/e2e-fixtures.ts +++ b/app/test/e2e/e2e-fixtures.ts @@ -29,6 +29,7 @@ import { createMockUpdateServer, MOCK_CONTROL_URL, type IMockUpdateServer, + type UpdateBehavior, } from './mock-update-server' import { getDistPath, getExecutableName } from '../../../script/dist-info' import { getProductName } from '../../package-info' @@ -110,7 +111,13 @@ export function terminateWindowsUpdaterProcesses() { // ── Helpers exposed to tests ──────────────────────────────────────── -export function controlMockServer(action: string): Promise { +type MockControlAction = + | `set-behavior/${UpdateBehavior}` + | 'reset-requests' + | 'requests' + | 'behavior' + +export function controlMockServer(action: MockControlAction): Promise { return new Promise((resolve, reject) => { http .get(`${MOCK_CONTROL_URL}/${action}`, res => { diff --git a/app/test/e2e/mock-update-server.ts b/app/test/e2e/mock-update-server.ts index 289f0c4c6e8..6c7a8e14276 100644 --- a/app/test/e2e/mock-update-server.ts +++ b/app/test/e2e/mock-update-server.ts @@ -34,7 +34,7 @@ function getWindowsUpdateAvailableReleases() { return `${fakeSha} ${nextWindowsPackageName} ${fakePackageSize}` } -type UpdateBehavior = 'no-update' | 'update-available' +export type UpdateBehavior = 'no-update' | 'update-available' export interface IMockUpdateServer { readonly server: http.Server From a66348c4146f8a91669281dee0cbde2733f4c36e Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Thu, 19 Mar 2026 17:54:34 +0100 Subject: [PATCH 15/26] Guard macOS-only dialog in e2e fixtures Add a platform check to dismissMoveToApplicationsDialog so it returns immediately when not running on Darwin (macOS). This prevents attempting to locate macOS-specific dialog buttons on other platforms during e2e tests. --- app/test/e2e/e2e-fixtures.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/test/e2e/e2e-fixtures.ts b/app/test/e2e/e2e-fixtures.ts index 3d8289d517f..79c1011858d 100644 --- a/app/test/e2e/e2e-fixtures.ts +++ b/app/test/e2e/e2e-fixtures.ts @@ -136,6 +136,10 @@ export async function getMockRequests(): Promise< } export async function dismissMoveToApplicationsDialog(page: Page) { + if (process.platform !== 'darwin') { + return + } + const btn = page.locator( 'button:has-text("Not Now"), button:has-text("Not now")' ) From f037d09b89e1246b9fe32da0b2373c20e13102d7 Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Fri, 20 Mar 2026 10:13:02 +0100 Subject: [PATCH 16/26] Delete default-to-tls12-on-appveyor.reg --- script/default-to-tls12-on-appveyor.reg | 7 ------- 1 file changed, 7 deletions(-) delete mode 100644 script/default-to-tls12-on-appveyor.reg diff --git a/script/default-to-tls12-on-appveyor.reg b/script/default-to-tls12-on-appveyor.reg deleted file mode 100644 index 9a1ae5cb253..00000000000 --- a/script/default-to-tls12-on-appveyor.reg +++ /dev/null @@ -1,7 +0,0 @@ -Windows Registry Editor Version 5.00 - -[HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\.NETFramework\v4.0.30319] -"SchUseStrongCrypto"=dword:00000001 - -[HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\Microsoft\.NETFramework\v4.0.30319] -"SchUseStrongCrypto"=dword:00000001 From deb80c824adc0d3e861a23bf30d578d5a5115a84 Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Fri, 20 Mar 2026 10:32:01 +0100 Subject: [PATCH 17/26] Add test setup step to CI workflow Insert a 'Prepare testing environment' step into .github/workflows/ci.yml to run `yarn test:setup` before the Windows signing action. The step passes npm_config_arch from the matrix to ensure the test environment is prepared for the correct architecture. --- .github/workflows/ci.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 42420c2e36d..92dba93cb72 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -191,6 +191,10 @@ jobs: KEY_PASSWORD: ${{ secrets.APPLE_APPLICATION_CERT_PASSWORD }} npm_config_arch: ${{ matrix.arch }} TARGET_ARCH: ${{ matrix.arch }} + - name: Prepare testing environment + run: yarn test:setup + env: + npm_config_arch: ${{ matrix.arch }} - uses: ./.github/actions/setup-windows-signing with: enabled: ${{ inputs.sign }} From e62514c054d58c10cb74e49e24bc90b1b9a70e2c Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Fri, 20 Mar 2026 10:42:22 +0100 Subject: [PATCH 18/26] Update app/test/e2e/app-launch.e2e.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- app/test/e2e/app-launch.e2e.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/test/e2e/app-launch.e2e.ts b/app/test/e2e/app-launch.e2e.ts index b264606a011..639242aac7f 100644 --- a/app/test/e2e/app-launch.e2e.ts +++ b/app/test/e2e/app-launch.e2e.ts @@ -3,7 +3,8 @@ * * These tests launch the real production-built app, interact with it * via Playwright, and verify core functionality end-to-end. Video and - * trace recording are enabled via playwright.config.ts. + * trace recording behavior is configured in the shared Electron fixtures + * (see ./e2e-fixtures), not directly in playwright.config.ts. */ import { From e51ee813cbca041c0a901fcf8682a8d98a13ade7 Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Fri, 20 Mar 2026 10:43:33 +0100 Subject: [PATCH 19/26] Update app/test/e2e/mock-update-server.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- app/test/e2e/mock-update-server.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/app/test/e2e/mock-update-server.ts b/app/test/e2e/mock-update-server.ts index 6c7a8e14276..1ce70cec3cd 100644 --- a/app/test/e2e/mock-update-server.ts +++ b/app/test/e2e/mock-update-server.ts @@ -58,10 +58,13 @@ export interface IMockUpdateServer { * * By default, it responds with "no update available" (HTTP 204). * - * In `update-available` mode, the JSON feed tells Squirrel an update - * exists. Squirrel will attempt to download the zip, receive a 404, and - * emit an `error` event. This is enough to verify that the app correctly - * processes the update feed and transitions through the expected states. + * In `update-available` mode, the JSON (macOS) or RELEASES (Windows) + * feed tells Squirrel that an update exists. When Squirrel then requests + * the update payload (zip or `.nupkg`), the mock server responds with + * HTTP 200 and intentionally keeps the download response open without + * completing. This is enough to verify that the app correctly processes + * the update feed and transitions into (and remains in) the expected + * "update available" / "downloading" states during tests. * * Full binary verification is not possible in dev builds because * Squirrel.Mac requires the update zip to satisfy the running app's code From a83f1162a68d48a2327ae7d94fc2acf0e5e2a837 Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Fri, 20 Mar 2026 10:43:45 +0100 Subject: [PATCH 20/26] Update app/test/e2e/e2e-fixtures.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- app/test/e2e/e2e-fixtures.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/test/e2e/e2e-fixtures.ts b/app/test/e2e/e2e-fixtures.ts index 79c1011858d..57fda73eb47 100644 --- a/app/test/e2e/e2e-fixtures.ts +++ b/app/test/e2e/e2e-fixtures.ts @@ -5,7 +5,7 @@ * * Provides: * - `app` — the ElectronApplication instance - * - `page` — the main BrowserWindow page + * - `mainWindow` — the main BrowserWindow page * - `mockServer` — the mock update server (with control helpers) * * All fixtures are scoped to the **worker** so the app launches once From c271865a8b4cbfd25708f16ff8f0da9de3d39a3f Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Fri, 20 Mar 2026 14:55:23 +0100 Subject: [PATCH 21/26] Run Windows signing action only on Windows runners Add runner.os == 'Windows' condition to the setup-windows-signing action in CI. This change wraps the action in an if check in two job steps so the Windows code-signing setup only runs on Windows runners, preventing it from executing on non-Windows environments. --- .github/workflows/ci.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 92dba93cb72..70af59a7031 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -124,7 +124,8 @@ jobs: run: yarn test:unit - name: Run script tests run: yarn test:script - - uses: ./.github/actions/setup-windows-signing + - if: runner.os == 'Windows' + uses: ./.github/actions/setup-windows-signing with: enabled: ${{ inputs.sign }} azure-client-id: ${{ secrets.AZURE_CODE_SIGNING_CLIENT_ID }} @@ -195,7 +196,8 @@ jobs: run: yarn test:setup env: npm_config_arch: ${{ matrix.arch }} - - uses: ./.github/actions/setup-windows-signing + - if: runner.os == 'Windows' + uses: ./.github/actions/setup-windows-signing with: enabled: ${{ inputs.sign }} azure-client-id: ${{ secrets.AZURE_CODE_SIGNING_CLIENT_ID }} From 6b74e82a323080856575cce78659dc3282bd4f28 Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Fri, 20 Mar 2026 14:58:58 +0100 Subject: [PATCH 22/26] Grant id-token write permission in CI workflow Add `id-token: write` to the job permissions in .github/workflows/ci.yml so the workflow can request OIDC tokens for cloud provider authentication and secure deployments. This change enables actions that exchange GitHub OIDC tokens (e.g., to assume roles) while keeping the permission scoped to the CI jobs. --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 70af59a7031..f32a4f0e3bb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -153,6 +153,7 @@ jobs: runs-on: ${{ matrix.os }} permissions: contents: read + id-token: write strategy: fail-fast: false matrix: From b8db93944a5674ebdcb16e4e73c116cfec0fe0ab Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Fri, 20 Mar 2026 15:43:56 +0100 Subject: [PATCH 23/26] Refactor E2E auto-update checks Add utilities to determine release channel and centralize 'Check for Updates' clicking in app-launch E2E tests. Tests now skip startup update checks for non-auto-update channels and reuse clickCheckForUpdatesIfAvailable to avoid duplicated locator logic and brittle checks. --- app/test/e2e/app-launch.e2e.ts | 69 ++++++++++++++++++++++++---------- 1 file changed, 50 insertions(+), 19 deletions(-) diff --git a/app/test/e2e/app-launch.e2e.ts b/app/test/e2e/app-launch.e2e.ts index 639242aac7f..82eb48d7012 100644 --- a/app/test/e2e/app-launch.e2e.ts +++ b/app/test/e2e/app-launch.e2e.ts @@ -23,7 +23,8 @@ import { getSmokeRepoHeadMessage, getSmokeRepoStatus, } from './test-helpers' -import type { Page } from '@playwright/test' +import { getVersion } from '../../package-info' +import type { Locator, Page } from '@playwright/test' // All tests run sequentially in the same Electron session. test.describe.configure({ mode: 'serial' }) @@ -58,6 +59,43 @@ function isMockUpdateRequest(url: string) { ) } +function getReleaseChannelForE2E() { + const configuredChannel = process.env.RELEASE_CHANNEL + + if (configuredChannel !== undefined) { + return configuredChannel + } + + const version = getVersion() + + if (version.includes('test')) { + return 'test' + } + + if (version.includes('beta')) { + return 'beta' + } + + return 'production' +} + +const releaseChannel = getReleaseChannelForE2E() +const shouldAutoCheckForUpdatesOnLaunch = + releaseChannel !== 'development' && releaseChannel !== 'test' + +async function clickCheckForUpdatesIfAvailable(target: Page | Locator) { + const checkBtn = target.locator( + 'button.button-component:has-text("Check for Updates")' + ) + + if ( + (await checkBtn.isVisible().catch(() => false)) && + (await checkBtn.isEnabled().catch(() => false)) + ) { + await checkBtn.click() + } +} + // ── Smoke tests ───────────────────────────────────────────────────── test.describe('GitHub Desktop - App Launch', () => { @@ -223,6 +261,11 @@ test.describe('Auto-update', () => { test('sends an update check to the mock server on launch', async ({ mockServer, }) => { + test.skip( + !shouldAutoCheckForUpdatesOnLaunch, + `Startup update checks are disabled for the ${releaseChannel} channel.` + ) + await expect .poll( async () => { @@ -260,6 +303,10 @@ test.describe('Auto-update', () => { test('shows up-to-date status after no-update check', async ({ mainWindow: page, }) => { + if (!shouldAutoCheckForUpdatesOnLaunch) { + await clickCheckForUpdatesIfAvailable(page) + } + const updateStatus = page.locator('#about .update-status') await updateStatus.waitFor({ state: 'visible', timeout: 10000 }) @@ -291,15 +338,7 @@ test.describe('Auto-update', () => { const aboutDialog = page.locator('#about') await aboutDialog.waitFor({ state: 'visible', timeout: 5000 }) - const checkBtn = aboutDialog.locator( - 'button.button-component:has-text("Check for Updates")' - ) - if ( - (await checkBtn.isVisible().catch(() => false)) && - (await checkBtn.isEnabled().catch(() => false)) - ) { - await checkBtn.click() - } + await clickCheckForUpdatesIfAvailable(aboutDialog) // Wait for status change const updateStatus = aboutDialog.locator('.update-status') @@ -346,15 +385,7 @@ test.describe('Auto-update', () => { const aboutDialog = page.locator('#about') await aboutDialog.waitFor({ state: 'visible', timeout: 5000 }) - const checkBtn = aboutDialog.locator( - 'button.button-component:has-text("Check for Updates")' - ) - if ( - (await checkBtn.isVisible().catch(() => false)) && - (await checkBtn.isEnabled().catch(() => false)) - ) { - await checkBtn.click() - } + await clickCheckForUpdatesIfAvailable(aboutDialog) const updateStatus = aboutDialog.locator('.update-status') await expect From 5de669d82a740d5382cbfa0b19fbbde0baf1b6c8 Mon Sep 17 00:00:00 2001 From: Sergio Padrino Date: Fri, 20 Mar 2026 18:08:09 +0100 Subject: [PATCH 24/26] Use expect.poll for update status in e2e test Replace a single textContent read and manual wait with Playwright's expect.poll to repeatedly check the About dialog update status. This makes the e2e check more robust by polling up to 15s (1s intervals) for the message "you have the latest version" in app/test/e2e/app-launch.e2e.ts. --- app/test/e2e/app-launch.e2e.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/app/test/e2e/app-launch.e2e.ts b/app/test/e2e/app-launch.e2e.ts index 82eb48d7012..213dc5c0f10 100644 --- a/app/test/e2e/app-launch.e2e.ts +++ b/app/test/e2e/app-launch.e2e.ts @@ -309,9 +309,14 @@ test.describe('Auto-update', () => { const updateStatus = page.locator('#about .update-status') await updateStatus.waitFor({ state: 'visible', timeout: 10000 }) - - const txt = await updateStatus.textContent() - expect(txt?.toLowerCase()).toContain('you have the latest version') + await expect + .poll( + async () => { + return ((await updateStatus.textContent()) ?? '').toLowerCase() + }, + { timeout: 15000, intervals: [1000] } + ) + .toContain('you have the latest version') }) test('closes the About dialog', async ({ mainWindow: page }) => { From 9f0982f52a7a3a0f64689ce9fd0740201c6494ec Mon Sep 17 00:00:00 2001 From: Pol Rivero <65060696+pol-rivero@users.noreply.github.com> Date: Tue, 24 Mar 2026 21:02:56 +0100 Subject: [PATCH 25/26] Fix e2e tests --- .github/workflows/ci.yml | 21 ++++++++++++++------- app/src/ui/branches/branch-list.tsx | 2 +- app/test/e2e/app-launch.e2e.ts | 5 +---- app/test/e2e/e2e-fixtures.ts | 2 +- script/testing-docker/entrypoint.sh | 1 - 5 files changed, 17 insertions(+), 14 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ebec079c282..e98fa820cf5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -278,9 +278,9 @@ jobs: - name: Install app on macOS if: runner.os == 'macOS' run: | - rm -rf "/Applications/GitHub Desktop.app" - ditto "dist/GitHub Desktop-darwin-arm64/GitHub Desktop.app" "/Applications/GitHub Desktop.app" - echo "DESKTOP_E2E_APP_PATH=/Applications/GitHub Desktop.app/Contents/MacOS/GitHub Desktop" >> "$GITHUB_ENV" + rm -rf "/Applications/GitHub Desktop Plus.app" + ditto "dist/GitHub Desktop Plus-darwin-arm64/GitHub Desktop Plus.app" "/Applications/GitHub Desktop Plus.app" + echo "DESKTOP_E2E_APP_PATH=/Applications/GitHub Desktop Plus.app/Contents/MacOS/GitHub Desktop Plus" >> "$GITHUB_ENV" - name: Install app on Windows if: runner.os == 'Windows' shell: pwsh @@ -288,7 +288,7 @@ jobs: function Write-SquirrelLogs { $logPaths = @( "$env:LOCALAPPDATA\SquirrelSetup.log", - "$env:LOCALAPPDATA\GitHubDesktop\SquirrelSetup.log" + "$env:LOCALAPPDATA\GitHubDesktopPlus\SquirrelSetup.log" ) foreach ($logPath in $logPaths) { @@ -299,7 +299,14 @@ jobs: } } - $setupExe = "dist/GitHubDesktopSetup-${{ matrix.arch }}.exe" + $setupExe = Get-ChildItem "dist/GitHubDesktopPlus-*-windows-${{ matrix.arch }}.exe" -ErrorAction SilentlyContinue | + Sort-Object FullName -Descending | + Select-Object -First 1 -ExpandProperty FullName + + if (-not $setupExe) { + throw "Unable to locate Windows installer executable" + } + $installer = Start-Process -FilePath $setupExe -ArgumentList "/S" -PassThru try { @@ -309,11 +316,11 @@ jobs: throw "Windows installer timed out after 300 seconds" } - Get-Process GitHubDesktop -ErrorAction SilentlyContinue | Stop-Process -Force + Get-Process GitHubDesktopPlus -ErrorAction SilentlyContinue | Stop-Process -Force $installedExe = $null for ($attempt = 0; $attempt -lt 30 -and -not $installedExe; $attempt++) { - $installedExe = Get-ChildItem "$env:LOCALAPPDATA\GitHubDesktop\app-*\GitHubDesktop.exe" -ErrorAction SilentlyContinue | + $installedExe = Get-ChildItem "$env:LOCALAPPDATA\GitHubDesktopPlus\app-*\GitHubDesktopPlus.exe" -ErrorAction SilentlyContinue | Sort-Object FullName -Descending | Select-Object -First 1 -ExpandProperty FullName diff --git a/app/src/ui/branches/branch-list.tsx b/app/src/ui/branches/branch-list.tsx index 890fbae62e2..b7d4203a3bc 100644 --- a/app/src/ui/branches/branch-list.tsx +++ b/app/src/ui/branches/branch-list.tsx @@ -373,7 +373,7 @@ export class BranchList extends React.Component { private onRenderNewButton = () => { return this.props.canCreateNewBranch ? ( - diff --git a/app/test/e2e/app-launch.e2e.ts b/app/test/e2e/app-launch.e2e.ts index 213dc5c0f10..359911dbf6d 100644 --- a/app/test/e2e/app-launch.e2e.ts +++ b/app/test/e2e/app-launch.e2e.ts @@ -252,10 +252,7 @@ test.describe('GitHub Desktop - App Launch', () => { // ── Auto-update tests ─────────────────────────────────────────────── test.describe('Auto-update', () => { - test.skip( - process.platform === 'win32' && !process.env.DESKTOP_E2E_APP_PATH, - 'Windows auto-update requires an installed Squirrel app, not a packaged app directory.' - ) + test.skip(true, 'Auto-update has been removed in GitHub Desktop Plus.') test.describe('startup update check', () => { test('sends an update check to the mock server on launch', async ({ diff --git a/app/test/e2e/e2e-fixtures.ts b/app/test/e2e/e2e-fixtures.ts index 57fda73eb47..e5d39a925b3 100644 --- a/app/test/e2e/e2e-fixtures.ts +++ b/app/test/e2e/e2e-fixtures.ts @@ -101,7 +101,7 @@ export function terminateWindowsUpdaterProcesses() { return } - for (const imageName of ['Update.exe', 'GitHubDesktop.exe']) { + for (const imageName of ['Update.exe', 'GitHubDesktopPlus.exe']) { spawnSync('taskkill', ['/F', '/T', '/IM', imageName], { stdio: 'ignore', windowsHide: true, diff --git a/script/testing-docker/entrypoint.sh b/script/testing-docker/entrypoint.sh index a25c25a9885..9a988734a25 100755 --- a/script/testing-docker/entrypoint.sh +++ b/script/testing-docker/entrypoint.sh @@ -6,7 +6,6 @@ set -e cd /app yarn test:unit -yarn test:eslint yarn test:script echo '-------------------' From c2f47e1a0809c35113efa6f4e4b7e3453d5f391a Mon Sep 17 00:00:00 2001 From: Pol Rivero <65060696+pol-rivero@users.noreply.github.com> Date: Tue, 24 Mar 2026 21:45:27 +0100 Subject: [PATCH 26/26] Update docker test environment to match CI --- script/testing-docker/Dockerfile | 4 ++-- script/testing-docker/entrypoint.sh | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/script/testing-docker/Dockerfile b/script/testing-docker/Dockerfile index 7c97bccf843..a817067288d 100644 --- a/script/testing-docker/Dockerfile +++ b/script/testing-docker/Dockerfile @@ -4,7 +4,7 @@ RUN apt-get -y update RUN apt-get -y install git make g++ curl libsecret-1-dev libnss3 libdbus-1-3 libatk1.0 libatk-bridge2.0-dev libcups2-dev libdrm-dev libgtk-3-dev libasound2-dev # Install Node.js -RUN curl -sL https://deb.nodesource.com/setup_22.x | bash - +RUN curl -sL https://deb.nodesource.com/setup_24.x | bash - RUN apt-get -y install nodejs RUN corepack enable @@ -18,6 +18,6 @@ RUN echo 'export PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/b COPY ./entrypoint.sh /entrypoint.sh # To allow the image to run without internet connection -RUN corepack pack yarn@1.21.1 +RUN corepack pack yarn@1.22.22 CMD ["/entrypoint.sh"] diff --git a/script/testing-docker/entrypoint.sh b/script/testing-docker/entrypoint.sh index 9a988734a25..ea524988ac6 100755 --- a/script/testing-docker/entrypoint.sh +++ b/script/testing-docker/entrypoint.sh @@ -5,6 +5,7 @@ set -e cd /app +yarn test:setup yarn test:unit yarn test:script