Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/cmt-provisioner.yml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ jobs:
'{owner:$owner, repo:$repo, sha:$sha, ref:$ref, run_id:$run_id, server_versions:$versions}')"

echo "Requesting CMT provisioning for server versions: ${SERVER_VERSIONS}"
echo "CMT dispatch payload: ${payload}"

curl -sS -X POST "${MATTERWICK_URL}/cmt_dispatch" \
-H "Content-Type: application/json" \
Expand Down
35 changes: 35 additions & 0 deletions .github/workflows/e2e-functional-template.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ on:
MM_SERVER_VERSION:
type: string
required: true
pr_number:
description: "When set, the Linux E2E job updates the PR body with Server for Cursor Automation URL"
required: false
type: string
default: ""
outputs:
NEW_FAILURES_LINUX:
description: "The output to comment"
Expand Down Expand Up @@ -140,6 +145,11 @@ on:
MM_SERVER_VERSION:
type: string
required: true
pr_number:
description: "When set, the Linux E2E job updates the PR body with Server for Cursor Automation URL"
required: false
type: string
default: ""

env:
BRANCH: ${{ github.head_ref || github.ref_name }}
Expand All @@ -159,6 +169,7 @@ jobs:
runs-on: ${{ inputs.runs-on }}
permissions:
contents: read
pull-requests: write
defaults:
run:
shell: bash
Expand Down Expand Up @@ -230,6 +241,30 @@ jobs:
with:
ref: ${{ inputs.DESKTOP_VERSION }}

- name: e2e/sync-cursor-automation-server-on-pr
if: ${{ runner.os == 'Linux' && inputs.pr_number != '' && inputs.MM_TEST_SERVER_URL != '' }}
continue-on-error: true
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
env:
SYNC_PR_NUMBER: ${{ inputs.pr_number }}
SYNC_SERVER_URL: ${{ inputs.MM_TEST_SERVER_URL }}
with:
github-token: ${{ github.token }}
script: |
const { syncCursorAutomationServerLine } = require('./e2e/utils/github-actions.js');
const prNumber = parseInt(process.env.SYNC_PR_NUMBER, 10);
if (!prNumber || Number.isNaN(prNumber)) {
core.info('sync-cursor-automation-server-on-pr: skipped (invalid pr_number)');
} else {
await syncCursorAutomationServerLine({
github,
owner: context.repo.owner,
repo: context.repo.repo,
prNumber,
serverUrl: process.env.SYNC_SERVER_URL,
});
}

- name: e2e/setup-node
uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0
with:
Expand Down
4 changes: 3 additions & 1 deletion .github/workflows/e2e-functional.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,13 @@ on:
default: ""
pr_number:
type: string
description: "PR number to remove E2E label from after tests complete"
description: "PR number for label cleanup; also used to write Server for Cursor Automation URL into the PR body (Linux E2E). Dispatchers should set this for PR runs."
required: false

permissions:
contents: read
statuses: write
pull-requests: write

jobs:
prepare-matrix:
Expand Down Expand Up @@ -92,6 +93,7 @@ jobs:
MM_TEST_PASSWORD: ${{ inputs.MM_TEST_PASSWORD }}
TYPE: ${{ inputs.run_type != '' && inputs.run_type || startsWith(inputs.version_name, 'release-') && 'RELEASE' || 'PR' }}
nightly: ${{ inputs.nightly }}
pr_number: ${{ inputs.pr_number }}
secrets: inherit

update-final-status:
Expand Down
6 changes: 6 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,12 @@ Modules extending `EventEmitter` broadcast state changes. Define event constants
- **E2E tests**: Playwright, in `e2e/` (separate `package.json`).
- **Test globals**: `__HASH_VERSION__`, `__IS_NIGHTLY_BUILD__`, `__IS_MAC_APP_STORE__`, `__DISABLE_GPU__`, `__SKIP_ONBOARDING_SCREENS__`, `__SENTRY_DSN__`

### Cursor automation (PR Mattermost server)

Linux E2E can add a line to the PR body: `Server for Cursor Automation: <https://…>`. For Cursor agents, keep **`MM_TEST_USER_NAME`** and **`MM_TEST_PASSWORD`** in Cursor env; the **server URL** is taken from that PR line when **`MM_TEST_SERVER_URL` is not set**.

Before tests run, `e2e/global-setup.ts` loads the PR description and sets `MM_TEST_SERVER_URL` from that line when possible. It needs a **PR number** from `MM_TEST_PR_NUMBER`, `GITHUB_PR_NUMBER`, `PR_NUMBER`, or `GITHUB_REF` (for example `refs/pull/3834/merge`). Cloud agents that do not receive GitHub-style variables should set **`MM_TEST_PR_NUMBER`** to the PR under test. It also needs **`GITHUB_REPOSITORY`** (`owner/repo`) or **`MM_TEST_GITHUB_OWNER`** and **`MM_TEST_GITHUB_REPO`** (unless **`gh`** can resolve the repo from the checkout), and either **`GITHUB_TOKEN`** / **`GH_TOKEN`** for the GitHub API or an authenticated **`gh`** CLI as a fallback. If `MM_TEST_SERVER_URL` is already set, it is left unchanged.

### Mocking singletons

Singletons use default exports. Mock them with `__esModule: true` + `default`:
Expand Down
12 changes: 12 additions & 0 deletions e2e/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,18 @@ Many server-backed specs require:
- `MM_TEST_USER_NAME`
- `MM_TEST_PASSWORD`

### Cursor automation (URL from PR, credentials from env)

When **`MM_TEST_SERVER_URL` is unset**, Playwright global setup (`global-setup.ts`) tries to read the PR description and take the URL from the line **`Server for Cursor Automation: <url>`** (the same line CI writes after Linux E2E). Set **`MM_TEST_USER_NAME`** and **`MM_TEST_PASSWORD`** in Cursor env only; you do not need to duplicate the server URL there if the PR has that line.

Requirements for auto-resolution:

- **PR number**: `MM_TEST_PR_NUMBER`, or `GITHUB_PR_NUMBER` / `PR_NUMBER`, or `GITHUB_REF` shaped like `refs/pull/123/merge` (set `MM_TEST_PR_NUMBER` when the cloud agent has no GitHub-style ref)
- **Repo**: `GITHUB_REPOSITORY` (`owner/repo`) or `MM_TEST_GITHUB_OWNER` + `MM_TEST_GITHUB_REPO`
- **Access**: `GITHUB_TOKEN` or `GH_TOKEN` for `api.github.com`, or a working authenticated **`gh`** CLI

If `MM_TEST_SERVER_URL` is already set, global setup does not overwrite it.

Do not remove skips or platform guards unless the test can actually run in the current environment.

Examples:
Expand Down
25 changes: 25 additions & 0 deletions e2e/global-setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,18 @@ import * as fs from 'fs';
import * as os from 'os';
import * as path from 'path';

import {resolveMmTestServerUrlFromPrIfNeeded} from './helpers/resolveMmTestServerUrlFromPr';

const E2E_PROCESS_REGISTRY = path.join(os.tmpdir(), 'mattermost-desktop-e2e-main-pids.txt');
const MACOS_DEFAULTS_SNAPSHOT = path.join(os.tmpdir(), 'mattermost-desktop-e2e-macos-defaults-snapshot.json');

function readMacOsDefault(domain: string, key: string): string | null {
try {
return execFileSync('defaults', ['read', domain, key], {encoding: 'utf8', stdio: ['pipe', 'pipe', 'pipe']}).trim();
} catch {
return null;
}
}

/**
* Disable macOS window-restoration (Resume) for the Electron binary used in tests.
Expand All @@ -27,6 +38,8 @@ export default async function globalSetup() {
// ignore stale registry cleanup failures
}

await resolveMmTestServerUrlFromPrIfNeeded();

if (process.platform === 'darwin') {
// Multiple bundle IDs may be involved: com.github.Electron (Electron binary
// launched directly) and the app's own bundle ID (when running signed builds).
Expand All @@ -45,6 +58,18 @@ export default async function globalSetup() {
}
}

// Snapshot system defaults we are about to override so global-teardown
// can restore them (or delete keys that did not exist before).
try {
const snapshot = {
LSQuarantine: readMacOsDefault('com.apple.LaunchServices', 'LSQuarantine'),
DialogType: readMacOsDefault('com.apple.CrashReporter', 'DialogType'),
};
fs.writeFileSync(MACOS_DEFAULTS_SNAPSHOT, JSON.stringify(snapshot), 'utf8');
} catch {
// Non-fatal — teardown will skip restore if file missing
}

// Apply system-level settings to suppress macOS dialogs that block
// Electron startup. These target system domains (LaunchServices,
// CrashReporter) rather than per-app bundle IDs.
Expand Down
38 changes: 38 additions & 0 deletions e2e/global-teardown.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,42 @@ import * as os from 'os';
import * as path from 'path';

const E2E_PROCESS_REGISTRY = path.join(os.tmpdir(), 'mattermost-desktop-e2e-main-pids.txt');
const MACOS_DEFAULTS_SNAPSHOT = path.join(os.tmpdir(), 'mattermost-desktop-e2e-macos-defaults-snapshot.json');

function restoreMacOsDefaultsSnapshot() {
if (process.platform !== 'darwin') {
return;
}
try {
if (!fs.existsSync(MACOS_DEFAULTS_SNAPSHOT)) {
return;
}
const raw = fs.readFileSync(MACOS_DEFAULTS_SNAPSHOT, 'utf8');
fs.rmSync(MACOS_DEFAULTS_SNAPSHOT, {force: true});
const snap = JSON.parse(raw) as {LSQuarantine: string | null; DialogType: string | null};

const restoreKey = (domain: string, key: string, previous: string | null) => {
try {
if (previous === null) {
execFileSync('defaults', ['delete', domain, key], {stdio: 'ignore'});
return;
}
if (previous === '0' || previous === '1') {
execFileSync('defaults', ['write', domain, key, '-bool', previous === '1' ? 'true' : 'false'], {stdio: 'pipe'});
return;
}
execFileSync('defaults', ['write', domain, key, '-string', previous], {stdio: 'pipe'});
} catch {
// best-effort restore
}
};

restoreKey('com.apple.LaunchServices', 'LSQuarantine', snap.LSQuarantine ?? null);
restoreKey('com.apple.CrashReporter', 'DialogType', snap.DialogType ?? null);
} catch {
// ignore
}
}

/**
* Kill any main Electron processes still running from this test suite.
Expand All @@ -16,6 +52,8 @@ const E2E_PROCESS_REGISTRY = path.join(os.tmpdir(), 'mattermost-desktop-e2e-main
* matching across unrelated Electron helper processes.
*/
export default async function globalTeardown() {
restoreMacOsDefaultsSnapshot();

let pids: number[] = [];
try {
if (fs.existsSync(E2E_PROCESS_REGISTRY)) {
Expand Down
142 changes: 142 additions & 0 deletions e2e/helpers/resolveMmTestServerUrlFromPr.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
// Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.

import {execFileSync} from 'child_process';

// eslint-disable-next-line @typescript-eslint/no-require-imports
const {parseCursorAutomationServerUrlFromBody} = require('../utils/github-actions') as {
parseCursorAutomationServerUrlFromBody: (body: string) => string | null;
};

function parsePrNumberFromGithubRef(): number | null {
const ref = process.env.GITHUB_REF?.trim();
if (!ref) {
return null;
}
const m = (/^refs\/pull\/(\d+)\//).exec(ref);
if (!m) {
return null;
}
const n = parseInt(m[1], 10);
return Number.isNaN(n) || n < 1 ? null : n;
}

function resolvePrNumber(): number | null {
const raw =
process.env.MM_TEST_PR_NUMBER?.trim() ||
process.env.GITHUB_PR_NUMBER?.trim() ||
process.env.PR_NUMBER?.trim();
if (raw) {
const n = parseInt(raw, 10);
if (Number.isNaN(n) || n < 1) {
// eslint-disable-next-line no-console
console.warn(`[e2e] Ignoring invalid PR number for MM_TEST_SERVER_URL resolution: ${raw}`);
return null;
}
return n;
}
return parsePrNumberFromGithubRef();
}

function resolveOwnerRepo(): {owner: string; repo: string} | null {
const full = process.env.GITHUB_REPOSITORY?.trim();
if (full && full.includes('/')) {
const i = full.indexOf('/');
return {owner: full.slice(0, i), repo: full.slice(i + 1)};
}
const owner = process.env.MM_TEST_GITHUB_OWNER?.trim();
const repo = process.env.MM_TEST_GITHUB_REPO?.trim();
if (owner && repo) {
return {owner, repo};
}
return null;
}

async function fetchPrBodyFromGitHubApi(owner: string, repo: string, prNumber: number): Promise<string | null> {
const token = process.env.GITHUB_TOKEN?.trim() || process.env.GH_TOKEN?.trim();
const headers: Record<string, string> = {
Accept: 'application/vnd.github+json',
'X-GitHub-Api-Version': '2022-11-28',
};
if (token) {
headers.Authorization = `Bearer ${token}`;
}
const apiUrl = `https://api.github.com/repos/${owner}/${repo}/pulls/${prNumber}`;
try {
const res = await fetch(apiUrl, {headers});
if (!res.ok) {
// eslint-disable-next-line no-console
console.warn(
`[e2e] GitHub API GET pulls/${prNumber} returned HTTP ${res.status}; will try gh CLI if available.`,
);
return null;
}
const data = (await res.json()) as {body?: string | null};
return data.body ?? null;
} catch (e) {
// eslint-disable-next-line no-console
console.warn('[e2e] GitHub API fetch failed:', e);
return null;
}
}

function fetchPrBodyFromGhCli(prNumber: number): string | null {
try {
const out = execFileSync(
'gh',
['pr', 'view', String(prNumber), '--json', 'body', '-q', '.body'],
{encoding: 'utf8', stdio: ['pipe', 'pipe', 'pipe'], maxBuffer: 10 * 1024 * 1024},
Comment on lines +83 to +88

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify gh supports explicit repo targeting and confirm current call omits it.
gh pr view --help | rg -n -- '-R|--repo'
rg -n -C2 'fetchPrBodyFromGhCli|gh[[:space:]]*[,)]|--json|\\.body' e2e/helpers/resolveMmTestServerUrlFromPr.ts

Repository: mattermost/desktop

Length of output: 872


🏁 Script executed:

#!/bin/bash
set -euo pipefail

file="e2e/helpers/resolveMmTestServerUrlFromPr.ts"
nl -ba "$file" | sed -n '1,220p'

Repository: mattermost/desktop

Length of output: 105


🏁 Script executed:

#!/bin/bash
set -euo pipefail

file="e2e/helpers/resolveMmTestServerUrlFromPr.ts"
# Narrow search for how repo is derived / passed
rg -n "ownerRepo|GITHUB_REPOSITORY|repository|owner\\/repo|--repo|-R" "$file" || true

Repository: mattermost/desktop

Length of output: 233


🏁 Script executed:

#!/bin/bash
set -euo pipefail

file="e2e/helpers/resolveMmTestServerUrlFromPr.ts"
cat -n "$file" | sed -n '1,220p'

Repository: mattermost/desktop

Length of output: 5899


🏁 Script executed:

#!/bin/bash
set -euo pipefail

file="e2e/helpers/resolveMmTestServerUrlFromPr.ts"
rg -n "ownerRepo|GITHUB_REPOSITORY|repository|owner/repo|--repo|-R" "$file" || true

Repository: mattermost/desktop

Length of output: 530


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n "fetchPrBodyFromGhCli\\(" -S . || true

Repository: mattermost/desktop

Length of output: 273


Scope gh pr view to the resolved repository

Fallback CLI lookup ignores the already-resolved ownerRepo and runs gh pr view <n> without -R/--repo, so it can read the wrong PR body (or fail) in detached/non-standard checkouts even when ownerRepo is known (e.g., at lines 113-120 with fetchPrBodyFromGhCli at 83-88). Add -R <owner>/<repo> when ownerRepo is available.

💡 Proposed fix
-function fetchPrBodyFromGhCli(prNumber: number): string | null {
+function fetchPrBodyFromGhCli(
+    prNumber: number,
+    ownerRepo?: {owner: string; repo: string},
+): string | null {
     try {
+        const args = ['pr', 'view', String(prNumber), '--json', 'body', '-q', '.body'];
+        if (ownerRepo) {
+            args.push('-R', `${ownerRepo.owner}/${ownerRepo.repo}`);
+        }
         const out = execFileSync(
             'gh',
-            ['pr', 'view', String(prNumber), '--json', 'body', '-q', '.body'],
+            args,
             {encoding: 'utf8', stdio: ['pipe', 'pipe', 'pipe'], maxBuffer: 10 * 1024 * 1024},
         );
         const body = out.trim();
         return body.length > 0 ? body : null;
     } catch {
         return null;
     }
 }
@@
-    if (!body) {
-        body = fetchPrBodyFromGhCli(prNumber);
+    if (!body) {
+        body = fetchPrBodyFromGhCli(prNumber, ownerRepo ?? undefined);
     }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@e2e/helpers/resolveMmTestServerUrlFromPr.ts` around lines 83 - 88, The GH CLI
call in fetchPrBodyFromGhCli currently invokes `gh pr view <pr>` without scoping
to the resolved repository, which can return the wrong PR body in detached
checkouts; update the execFileSync args in `fetchPrBodyFromGhCli` to include
`-R` (or `--repo`) with the resolved `ownerRepo` when `ownerRepo` is available
(i.e., prepend/insert `-R, ownerRepo` into the arguments array passed to
`execFileSync`), ensuring the function uses the `ownerRepo` variable to target
the correct repository.

);
const body = out.trim();
return body.length > 0 ? body : null;
} catch {
return null;
}
}

/**
* When MM_TEST_SERVER_URL is unset, load it from the PR body line
* "Server for Cursor Automation: <url>" (same format CI writes). Requires a PR
* number (MM_TEST_PR_NUMBER, GITHUB_PR_NUMBER, PR_NUMBER, or GITHUB_REF=refs/pull/N/…)
* and either GITHUB_REPOSITORY, MM_TEST_GITHUB_OWNER+REPO, or a working `gh` CLI.
*/
export async function resolveMmTestServerUrlFromPrIfNeeded(): Promise<void> {
if (process.env.MM_TEST_SERVER_URL?.trim()) {
return;
}

const prNumber = resolvePrNumber();
if (!prNumber) {
return;
}

const ownerRepo = resolveOwnerRepo();
let body: string | null = null;
if (ownerRepo) {
body = await fetchPrBodyFromGitHubApi(ownerRepo.owner, ownerRepo.repo, prNumber);
}
if (!body) {
body = fetchPrBodyFromGhCli(prNumber);
}

if (!body) {
// eslint-disable-next-line no-console
console.warn(
`[e2e] Could not load PR #${prNumber} body (set GITHUB_REPOSITORY + GITHUB_TOKEN, or install auth'd gh). MM_TEST_SERVER_URL remains unset.`,
);
return;
}

const url = parseCursorAutomationServerUrlFromBody(body);
if (url) {
process.env.MM_TEST_SERVER_URL = url;
// eslint-disable-next-line no-console
console.log('[e2e] MM_TEST_SERVER_URL set from PR body (Server for Cursor Automation line).');
return;
}

// eslint-disable-next-line no-console
console.warn(
`[e2e] PR #${prNumber} has no usable "Server for Cursor Automation:" URL line; MM_TEST_SERVER_URL remains unset.`,
);
}
15 changes: 8 additions & 7 deletions e2e/specs/menu_bar/full_screen.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,15 @@ test.describe('menu/view', () => {
const toggleItem = viewMenu?.submenu?.items?.find(
(item: any) => item.role === 'togglefullscreen' || item.accelerator === 'F11',
);
if (toggleItem) {
const refs = (global as any).__e2eTestRefs;
const targetWindow = BrowserWindow.getFocusedWindow() ??
refs?.MainWindow?.get?.() ??
BrowserWindow.getAllWindows().find((w) => !w.isDestroyed()) ??
null;
toggleItem.click(undefined, targetWindow, undefined);
if (!toggleItem) {
throw new Error('exit fullscreen menu item not found');
}
const refs = (global as any).__e2eTestRefs;
const targetWindow = BrowserWindow.getFocusedWindow() ??
refs?.MainWindow?.get?.() ??
BrowserWindow.getAllWindows().find((w) => !w.isDestroyed()) ??
null;
toggleItem.click(undefined, targetWindow, undefined);
});

await electronApp.evaluate(async ({BrowserWindow}) => {
Expand Down
Loading
Loading