-
Notifications
You must be signed in to change notification settings - Fork 813
Open
Labels
needs-area-labelAn area label is needed to ensure this gets routed to the appropriate area ownersAn area label is needed to ensure this gets routed to the appropriate area owners
Description
Problem
The cli-e2e-recording-comment.yml workflow uses a hardcoded list of test class names to filter which artifacts to download for recordings:
const cliE2eArtifacts = allArtifacts.filter(a =>
a.name.startsWith('logs-') &&
a.name.includes('Tests-ubuntu-latest') &&
(a.name.includes('SmokeTests') ||
a.name.includes('EmptyAppHostTemplateTests') ||
a.name.includes('JsReactTemplateTests') ||
// ... etc
)
);When a PR adds a new E2E test class, the recording won't appear in the PR comment because:
- The
workflow_runtrigger always runs the workflow file from the default branch (main), not from the PR branch - This is a GitHub security feature to prevent PRs from modifying workflow behavior
- So even if the PR adds the new test class to the filter, the running workflow uses
main's version which doesn't have it
Impact
- First PR adding a new E2E test class won't have its recording posted
- Recording only works after the PR is merged and a subsequent PR runs
Proposed Fix
Replace the explicit list with a pattern-based filter:
// Before (explicit list)
a.name.includes('SmokeTests') || a.name.includes('EmptyAppHostTemplateTests') || ...
// After (pattern-based)
/^logs-.*Tests-ubuntu-latest$/.test(a.name)This matches any CLI E2E test artifact without needing to maintain a list.
Example
PR #14048 added PsCommandTests - the recording artifact exists but won't be posted until merged.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
needs-area-labelAn area label is needed to ensure this gets routed to the appropriate area ownersAn area label is needed to ensure this gets routed to the appropriate area owners