Skip to content

Fix CI detection in tests#659

Merged
SteveSandersonMS merged 4 commits intomainfrom
stevesa/fix-ci-detection
Mar 4, 2026
Merged

Fix CI detection in tests#659
SteveSandersonMS merged 4 commits intomainfrom
stevesa/fix-ci-detection

Conversation

@SteveSandersonMS
Copy link
Contributor

Test-only change to fix how we detect whether we're running in CI.

@SteveSandersonMS SteveSandersonMS requested a review from a team as a code owner March 4, 2026 10:55
Copilot AI review requested due to automatic review settings March 4, 2026 10:55
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the test harness CI-detection logic to avoid treating VS Code Vitest extension runs as “real CI”, while preserving strict behavior in actual CI where missing snapshots should fail.

Changes:

  • Refines CI detection to exclude VITEST_VSCODE runs even when CI=true.
  • Reuses the new isCI helper in Node E2E tests to decide when to inject the fake GitHub token.
  • Updates the replaying CAPI proxy behavior to only hard-fail on missing cached responses in real CI.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
test/harness/replayingCapiProxy.ts Uses refined isCI logic before failing on missing cached responses.
nodejs/test/e2e/session.test.ts Switches CI-dependent token injection to use shared isCI.
nodejs/test/e2e/harness/sdkTestContext.ts Introduces/export isCI and applies it to client setup for E2E tests.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 4, 2026

Cross-SDK Consistency Review ✅

I've reviewed this PR for cross-SDK consistency. No consistency issues found.

Summary

This PR fixes CI detection in the Node.js/TypeScript tests only, which is appropriate because:

  1. Language-specific issue: The VS Code Vitest extension sets CI=true when running tests locally in non-debug mode, causing false positives. This only affects TypeScript/JavaScript tests using Vitest.

  2. Other SDKs are not affected:

    • Python uses pytest (no VS Code Vitest extension)
    • Go uses native Go testing (no VS Code Vitest extension)
    • .NET uses xUnit (no VS Code Vitest extension)
  3. Correctly scoped changes: The fix is applied to:

    • nodejs/test/e2e/harness/sdkTestContext.ts - Node.js test context
    • nodejs/test/e2e/session.test.ts - Node.js E2E tests
    • test/harness/replayingCapiProxy.ts - Shared TypeScript proxy (used by Node.js tests)

Verification

I confirmed that other SDKs use simple CI == "true" checks:

  • python/e2e/testharness/context.py:63
  • python/e2e/test_session.py:186
  • go/internal/e2e/testharness/context.go:169
  • dotnet/test/Harness/E2ETestContext.cs:97

This is correct because they don't have the VS Code Vitest extension interference issue.

Recommendation: ✅ Approve - this change maintains proper cross-SDK consistency by fixing a language-specific testing issue without requiring changes to other SDKs.

AI generated by SDK Consistency Review Agent

@github-actions
Copy link
Contributor

github-actions bot commented Mar 4, 2026

Cross-SDK Consistency Review

I've reviewed this PR for consistency across all four SDK implementations (Node.js/TypeScript, Python, Go, and .NET).

✅ Consistency Issue Identified

This PR updates CI detection from process.env.CI === "true" to process.env.GITHUB_ACTIONS === "true" in the Node.js test files and the shared test harness (test/harness/replayingCapiProxy.ts).

However, the Python, Go, and .NET E2E test files still use the old CI environment variable pattern:

Python (python/e2e/testharness/context.py:63 and python/e2e/test_session.py:186):

github_token = "fake-token-for-e2e-tests" if os.environ.get("CI") == "true" else None

Go (go/internal/e2e/testharness/context.go:169):

if os.Getenv("CI") == "true" {
    options.GitHubToken = "fake-token-for-e2e-tests"
}

.NET (dotnet/test/Harness/E2ETestContext.cs:97 and :103):

GitHubToken = !string.IsNullOrEmpty(Environment.GetEnvironmentVariable("CI")) ? "fake-token-for-e2e-tests" : null,

and

var isCI = !string.IsNullOrEmpty(Environment.GetEnvironmentVariable("CI"));

📋 Recommendation

For consistency, consider updating the other SDKs to use the same GITHUB_ACTIONS environment variable check:

  • Python: os.environ.get("GITHUB_ACTIONS") == "true"
  • Go: os.Getenv("GITHUB_ACTIONS") == "true"
  • .NET: Environment.GetEnvironmentVariable("GITHUB_ACTIONS") == "true"

This would ensure all SDKs detect CI environments using the same, more reliable GitHub Actions-specific environment variable.

AI generated by SDK Consistency Review Agent

@SteveSandersonMS SteveSandersonMS merged commit 427205f into main Mar 4, 2026
35 checks passed
@SteveSandersonMS SteveSandersonMS deleted the stevesa/fix-ci-detection branch March 4, 2026 12:03
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