Skip to content

Skip VideoRecorder test on CI due to access denied errors#15587

Merged
nohwnd merged 1 commit intomicrosoft:mainfrom
nohwnd:skip-video-ci
Mar 26, 2026
Merged

Skip VideoRecorder test on CI due to access denied errors#15587
nohwnd merged 1 commit intomicrosoft:mainfrom
nohwnd:skip-video-ci

Conversation

@nohwnd
Copy link
Copy Markdown
Member

@nohwnd nohwnd commented Mar 26, 2026

Workaround for #15586 — video recording test fails with access denied on the official CI pipeline. Skips with \Assert.Inconclusive\ when \TF_BUILD\ is set, matching the existing pattern from \BlameDataCollectorTests.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

Copilot AI review requested due to automatic review settings March 26, 2026 11:05
Copy link
Copy Markdown
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

Skips the VideoRecorder acceptance test on CI to work around “access denied” failures during video recording (linked to #15586).

Changes:

  • Adds an early Assert.Inconclusive(...) guard intended to skip the test on CI.

Comment on lines +20 to +24
// Workaround for #15586 — video recording fails with access denied on the official CI pipeline.
if (Environment.GetEnvironmentVariable("_RunAsInternal") == "True")
{
Assert.Inconclusive("Video recorder test is skipped on the official pipeline due to access denied errors. See #15586.");
}
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

The PR description says the test should be skipped when TF_BUILD is set (matching BlameDataCollectorTests), but the code checks _RunAsInternal == \"True\". This looks inconsistent and may fail to skip on the official CI (or skip in the wrong environment). Either update the condition to use TF_BUILD (or the same predicate as BlameDataCollectorTests), or update the PR description/comment to reflect the actual gating signal.

Copilot uses AI. Check for mistakes.
public void VideoRecorderDataCollectorShouldRecordVideoWithRunSettings(RunnerInfo runnerInfo)
{
// Workaround for #15586 — video recording fails with access denied on the official CI pipeline.
if (Environment.GetEnvironmentVariable("_RunAsInternal") == "True")
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

Comparing environment variables with a case-sensitive literal can be brittle across environments/agents. Prefer a case-insensitive comparison (e.g., string.Equals(..., StringComparison.OrdinalIgnoreCase)) or a non-empty check if the variable is treated as a boolean flag.

Copilot uses AI. Check for mistakes.
Detect official pipeline via _RunAsInternal env variable (already set in
azure-pipelines-official.yml). Only skips on the specific pipeline where
the issue occurs, not on all CI or local runs.

Workaround for microsoft#15586

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@nohwnd nohwnd merged commit 65954d1 into microsoft:main Mar 26, 2026
1 of 4 checks passed
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