Skip to content

fix(SDK-6463): robust TS config compile in NX monorepos + tolerate a11y afterEach scan timeouts#1131

Open
Bhargavi-BS wants to merge 3 commits into
masterfrom
sdk-6463-nx-tsconfig-and-a11y-afterEach-fixes
Open

fix(SDK-6463): robust TS config compile in NX monorepos + tolerate a11y afterEach scan timeouts#1131
Bhargavi-BS wants to merge 3 commits into
masterfrom
sdk-6463-nx-tsconfig-and-a11y-afterEach-fixes

Conversation

@Bhargavi-BS

Copy link
Copy Markdown
Collaborator

SDK-6463 — Cypress test-count / status inconsistencies (customer: Sapiens, NX monorepo)

Fixes two client-side browserstack-cypress-cli defects surfaced by SDK-6463. The customer's reported dashboard count discrepancy is largely not a CLI bug (see RCA below); these two fixes address the genuine CLI defects: silently broken parallelization in NX monorepos, and accessibility-hook timeouts skipping tests.

RCA — the "inconsistency" is three layers

  • A. Test Observability rendering (grep-filtered tests shown as "Pending", timeout placeholders, new-dashboard counting) — faithful; owned by the TO/dashboard team, not the CLI.
  • B. Client-side test selection (@cypress/grep tags + browserstack.json exclude globs) — expected behavior.
  • C. Two real CLI defects — fixed here.

C.1 — TS cypress-config compile fails silently in NX monorepos → parallelization collapses

readCypressConfigUtil.convertTsConfig compiles the TS config to read it. In an NX/monorepo:

  • the extends temp tsconfig inherited base options (noEmit / emitDeclarationOnly / composite / noEmitOnError) that suppress or redirect the compiled JS, and
  • any tsc type error made tsc exit non-zero, which short-circuited the &&-chained tsc-alias, leaving path aliases (e.g. @org/lib) un-rewritten so the compiled config couldn't be require()d.

The read error was then silently swallowed (readCypressConfigFile catch) → cypressConfigFile = {}getNumberOfSpecFiles fell back to the default cypress/e2e/** glob → 0 specssetParallels forced parallels to browserCombinations.length. This is the customer's Parallels limit specified: 1 (despite parallels: 3) and the repeated Cypress config file not found at: …tmpBstackCompiledJs/… errors.

Fix (bin/helpers/readCypressConfigUtil.js):

  • Force emit-friendly overrides in the extends temp tsconfig: noEmit:false, emitDeclarationOnly:false, composite:false, declaration:false, declarationMap:false, noEmitOnError:false, incremental:false. These only affect the throwaway temp tsconfig used to read the config — not the user's real build.
  • Run tsc-alias unconditionally (& on Windows / ; on Unix instead of &&) so a tsc type error doesn't leave aliases un-rewritten. convertTsConfig already tolerates tsc errors by parsing the emitted-files output.

C.2 — accessibility afterEach scan/save timeout skips the rest of the spec

bin/accessibility-automation/cypress/index.js calls cy.wrap(performScan(win), {timeout:30000}) and cy.wrap(saveTestResults(win, …), {timeout:30000}) in the global afterEach with no rejection handling. When the in-page scanner doesn't echo A11Y_SCAN_FINISHED / A11Y_RESULTS_SAVED, the wrap times out, the afterEach hook fails, and Cypress skips all remaining tests in the spec — the ticket's "cy.wrap() timed out … Because this error occurred during a after each hook we are skipping all of the remaining tests" and "should have failed but got skipped".

Fix: add .catch to both wrap chains so a hung scan/save is logged, not cascaded into skipped tests.

Validation

C.1 — real BrowserStack infra (same project & config, only the CLI differs):

Unfixed CLI Fixed CLI
Cypress config file not found errors 2 0
Parallels limit specified (config has parallels: 3) 1 3
Build 7493f8339065f0570cd0867d7bec599487bb6dd9 bba6009df5e214058e1567f17ac268ea312675e0

C.1 — local NX tsconfig matrix: noEmit / emitDeclarationOnly / composite / noEmitOnError+type-error all fail to read the config before the fix and read OK after it.

C.2 — local simulation: drives the real plugin afterEach with a hung scanner; the new regression test fails without the fix and passes with it across hung-scan, hung-save, and happy-path scenarios.

Unit suite: 685 passing (was 678 → +7 new tests), 0 new failures (13 failures pre-exist on a clean tree and are unrelated).

Files

  • bin/helpers/readCypressConfigUtil.js — C.1 fix
  • bin/accessibility-automation/cypress/index.js — C.2 fix
  • test/unit/bin/helpers/readCypressConfigUtil.js — +3 C.1 regression tests
  • test/unit/bin/accessibility-automation/cypress/index.js — new, +4 C.2 regression tests

Notes / follow-ups

  • The dashboard count discrepancy (Layer A) should be routed to the Test Observability/dashboard team — it is not addressed here.
  • Latent bug spotted while investigating C.2: index.js:292 references an undefined includeTags (should be includeTagArray), reachable only when accessibility include/exclude tags are set. Not fixed here; worth a separate ticket.

🤖 Generated with Claude Code

Bhargavi-BS and others added 2 commits June 19, 2026 14:50
When the cypress config is TypeScript and lives in an NX/monorepo, the extends temp tsconfig inherited base options (noEmit / emitDeclarationOnly / composite / noEmitOnError) that suppress or redirect the compiled JS, and any tsc type-error short-circuited the '&&'-chained tsc-alias so path aliases were left un-rewritten. The compiled config then could not be found/required, the error was silently swallowed, and getNumberOfSpecFiles fell back to a default glob that found 0 specs -> setParallels collapsed parallels to 1.

Force emit-friendly overrides in the extends temp tsconfig and run tsc-alias unconditionally (& / ; instead of &&). Adds regression tests covering the emit-override and unconditional-alias behavior.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…K-6463)

A hung accessibility scan made the 30s cy.wrap() time out and fail the afterEach hook, which makes Cypress skip ALL remaining tests in the spec (they surface as 'skipped' instead of running). Add .catch handlers to both cy.wrap(..., {timeout:30000}) chains (performScan and saveTestResults) so a timeout is logged instead of cascading into skipped tests. Adds a regression test that loads the real plugin afterEach and asserts tolerance.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@Bhargavi-BS Bhargavi-BS requested a review from a team as a code owner June 19, 2026 09:22

@amaanbs amaanbs left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Automated SDK PR Review

Verdict: Request Changes — 1 finding to address before merging (W1), 1 suggestion for follow-up (S1).

Summary: 0 critical · 1 warning · 1 suggestion across 2 files reviewed.

The Unix half of the fix (; instead of && between tsc and tsc-alias) is correct and ready to ship. The Windows half (&) has a concurrency concern — on Windows CMD, & starts the second command without waiting for the first to finish, which may cause tsc-alias to process a not-yet-emitted file. This is verifiable empirically; see W1 for the idiomatic Windows fix. S1 (silent a11y scan timeout) is a follow-up suggestion, not a blocker.

See inline comments below for full Problem and Suggested Fix detail.

Generated by Automated SDK PR review.

// leave path aliases (e.g. @org/lib) un-rewritten -> the compiled config fails to
// require -> "Cypress config file not found" (SDK-6463). convertTsConfig already
// tolerates tsc errors by parsing the emitted-files output.
const tscCommand = `${setNodePath} && node "${typescript_path}" --project "${tempTsConfigPath}" & ${setNodePath} && node "${tsc_alias_path}" --project "${tempTsConfigPath}" --verbose`;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

⚠️ Warning — [FEATURE/FIX CORRECTNESS] Windows & introduces parallel execution, not sequential-unconditional

Problem

On Windows CMD, cmd1 & cmd2 executes both commands concurrentlycmd2 starts immediately without waiting for cmd1 to finish. This is the opposite of Unix ;, which is strictly sequential.

In this context: if tsc-alias starts before tsc has written the compiled .js to tmpBstackCompiledJs/, it processes a not-yet-emitted file — the alias rewrite is a silent no-op, and the compiled config still can't be require()d.

The intent stated in the comment is "run tsc-alias regardless of tsc's exit code, but after tsc completes." & satisfies "regardless of exit code" but not "after tsc completes."

Unix ; (the else branch) is exactly right. Windows & is not the equivalent.

Suggested Fix

Replace & with || cd. (a no-op fallback that preserves sequencing):

// Windows: '|| cd.' is the CMD idiom for "run B sequentially, regardless of A's exit code"
const tscCommand = `${setNodePath} && node "${typescript_path}" --project "${tempTsConfigPath}" || cd. && ${setNodePath} && node "${tsc_alias_path}" --project "${tempTsConfigPath}" --verbose`;

Or, if you prefer PowerShell (more readable, avoids CMD-ism):

const tscCommand = `powershell -Command "& '${typescript_path}' --project '${tempTsConfigPath}'; & '${tsc_alias_path}' --project '${tempTsConfigPath}' --verbose"`;

If you've empirically verified that & doesn't race in practice (e.g., because CMD buffers background-job start in non-interactive mode), add a code comment documenting that and the warning is acknowledged.

Confidence: 🟢 Objectively verifiable — Windows CMD & operator semantics are documented; the race is reproducible by running both commands in CMD with a slow tsc invocation.

} catch (er) {
browserStackLog(`Error in saving results with error: ${er.message}`);
}
}).catch((err) => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

💡 Suggestion — [GRACEFUL DEGRADATION] Silent scan timeout gives zero user feedback on Accessibility dashboard

Problem

The .catch is the right call here — preventing a hung scan from skipping remaining tests is the correct priority. However, the catch only logs via browserStackLog, which is suppressed when BROWSERSTACK_LOGS: false (a common CI configuration in Cypress.env).

When a scan times out under that configuration: the test passes, no accessibility results are recorded, and the Accessibility dashboard shows no data for that test — with no indication of why it's missing.

Suggested Fix

Consider also emitting a lightweight signal when the catch fires, e.g. via cy.task:

}).catch((err) => {
    browserStackLog(`Accessibility afterEach: scan timed out or failed: ${err && err.message}`);
    // Optionally: surface a "scan skipped" marker to the Accessibility dashboard
    // so it shows "scan skipped — timeout" instead of silently missing data.
    // cy.task('bstack:accessibility_scan_skipped', { reason: err && err.message }, { log: false });
})

Not a blocker — the current tradeoff (graceful degradation over partial data) is intentional and valid. Worth a follow-up ticket if the Accessibility team wants to surface timeout counts on the dashboard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants