-
Notifications
You must be signed in to change notification settings - Fork 19
fix: repair post-release dev-preview NUTs (W-23400737) #676
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,7 @@ import { spawn, ChildProcessByStdio, ChildProcessWithoutNullStreams } from 'node | |
| import { Readable } from 'node:stream'; | ||
| import type { Writable } from 'node:stream'; | ||
| import { type ChildProcess } from 'node:child_process'; | ||
| import fs from 'node:fs'; | ||
| import path from 'node:path'; | ||
| import { fileURLToPath } from 'node:url'; | ||
| import { TestSession } from '@salesforce/cli-plugins-testkit'; | ||
|
|
@@ -28,6 +29,26 @@ const MAX_WAIT_MS = 30_000; | |
|
|
||
| export const PLUGIN_ROOT_PATH = path.resolve(CURRENT_DIR_PATH, '../../../../..'); | ||
|
|
||
| /** | ||
| * Returns the highest org API version the plugin ships a dev server for, read from | ||
| * package.json's `apiVersionMetadata`. Scratch orgs are provisioned on whatever API | ||
| * version the platform currently defaults to, which drifts ahead of the plugin; the | ||
| * dev server rejects unsupported versions and exits before printing a preview URL. | ||
| * Pinning the preview to this version keeps the e2e tests decoupled from that drift | ||
| * and auto-tracks new versions as they are added to package.json. | ||
| * | ||
| * @returns The max supported API version string (e.g. '67.0'). | ||
| */ | ||
| export function getMaxSupportedApiVersion(): string { | ||
| const pkgPath = path.join(PLUGIN_ROOT_PATH, 'package.json'); | ||
| const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf8')) as { apiVersionMetadata?: Record<string, unknown> }; | ||
| const versions = Object.keys(pkg.apiVersionMetadata ?? {}); | ||
| if (versions.length === 0) { | ||
| throw new Error(`No apiVersionMetadata found in ${pkgPath}`); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: should we try a default version?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Intentional fail-fast here rather than a default. There's also no existing "default API version" constant anywhere in the source to fall back to, so a default would have to be a hardcoded magic number (e.g. Happy to revisit if we ever introduce a canonical default version the plugin resolves against. |
||
| } | ||
| return versions.sort((a, b) => parseFloat(b) - parseFloat(a))[0]; | ||
| } | ||
|
|
||
| /** | ||
| * Extracts the first LWR preview URL from process output. | ||
| * | ||
|
|
@@ -150,7 +171,11 @@ export function startLightningDevServer( | |
| OPEN_BROWSER: 'false', | ||
| LIGHTNING_DEV_PRINT_PREVIEW_URL: 'true', | ||
| }; | ||
| const args = [runJs, 'lightning', 'dev', 'component', '-o', username]; | ||
| // Pin the preview to the plugin's max supported API version. Scratch orgs are | ||
| // created on the platform's current default API version, which can be newer than | ||
| // any version the plugin bundles a dev server for; without this the dev server | ||
| // exits with an "unsupported API version" error and never prints a preview URL. | ||
| const args = [runJs, 'lightning', 'dev', 'component', '-o', username, '--api-version', getMaxSupportedApiVersion()]; | ||
| if (componentName) { | ||
| args.push('--name', componentName); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,23 +22,54 @@ let cachedSession: TestSession; | |
|
|
||
| const PROJECT_PATH = path.resolve(PLUGIN_ROOT_PATH, 'test/projects/component-preview-project'); | ||
|
|
||
| // Number of times TestSession will retry scratch org creation before failing. Scratch org | ||
| // signup is intermittently flaky (RemoteOrgSignupFailed / C-9999); retrying avoids spurious | ||
| // failures in the post-release pipeline. Overridable via TESTKIT_SETUP_RETRIES. | ||
| const SETUP_RETRIES = Number.parseInt(process.env.TESTKIT_SETUP_RETRIES ?? '', 10) || 3; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Curious use of
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Switched to the global |
||
|
|
||
| /** | ||
| * Restores process.cwd() if it is currently a leaked sinon stub. | ||
| * | ||
| * TestSession's constructor stubs process.cwd() *before* it creates scratch orgs. If org | ||
| * creation then throws, TestSession.create() rejects without ever returning the instance, | ||
| * so the sandbox is never restored and the stub leaks. The next file's TestSession.create() | ||
| * then throws a misleading "Attempted to wrap cwd which is already wrapped", masking the real | ||
| * error across every subsequent file. Restoring the stub here lets the true failure surface | ||
| * in the one file that actually failed. | ||
| */ | ||
| function restoreLeakedCwdStub(): void { | ||
| // A sinon stub is self-bound, so calling restore() off the proxy is safe here. | ||
| // eslint-disable-next-line @typescript-eslint/unbound-method | ||
| const cwd = process.cwd as typeof process.cwd & { isSinonProxy?: boolean; restore?: () => void }; | ||
| if (cwd.isSinonProxy) { | ||
| cwd.restore?.(); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Returns a shared TestSession for NUTs, created once and reused (same project and Dev Hub). | ||
| * | ||
| * @returns Promise that resolves to the cached or newly created TestSession. | ||
| */ | ||
| export async function getSession(): Promise<TestSession> { | ||
| if (!cachedSession) { | ||
| cachedSession = await TestSession.create({ | ||
| devhubAuthStrategy: 'AUTO', | ||
| project: { sourceDir: PROJECT_PATH }, | ||
| scratchOrgs: [ | ||
| { | ||
| config: 'config/project-scratch-def.json', | ||
| setDefault: true, | ||
| }, | ||
| ], | ||
| }); | ||
| try { | ||
| cachedSession = await TestSession.create({ | ||
| devhubAuthStrategy: 'AUTO', | ||
| project: { sourceDir: PROJECT_PATH }, | ||
| retries: SETUP_RETRIES, | ||
| scratchOrgs: [ | ||
| { | ||
| config: 'config/project-scratch-def.json', | ||
| setDefault: true, | ||
| }, | ||
| ], | ||
| }); | ||
| } catch (err) { | ||
| // Prevent a single setup failure from cascading into misleading errors in later files. | ||
| restoreLeakedCwdStub(); | ||
| throw err; | ||
| } | ||
| } | ||
| return new Promise((r) => r(cachedSession)); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not used outside this file, doesn't need to be exported
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch — addressed in follow-up #677.
getAccessTokenis now non-exported (it's only used bygetPreviewin this file).