Skip to content

Test hardening: runs boot execution in PlatformValidation - #40563

Open
shivangirana20 wants to merge 4 commits into
mainfrom
dev/shivangirana/hardenc9a46419
Open

shivangirana20 wants to merge 4 commits into
mainfrom
dev/shivangirana/hardenc9a46419

Conversation

@shivangirana20

@shivangirana20 shivangirana20 commented Oct 29, 2026 •

Copy link
Copy Markdown
Member

Test Hardening

Test Information

Test Method: seven catalog/discovery/lifecycle scenarios plus fourteen safeguards
Test Class: PlatformValidation, PlatformValidation lifecycle safeguards
Test Project: @azure/arm-platformvalidation
Test File Path: sdk/platformvalidation/arm-platformvalidation/test/public/

Hardening Categories Applied

  • Assertion strength
  • Edge-case coverage
  • Stability-by-design
  • Parameterization
  • Naming and intent
  • Mock saturation

Audit Findings Applied

  • Assert strict subscription-scoped catalog, manifest and result identities with one passed boot test and no failed/skipped results.
  • Require credential-free fixture inputs and generate UUID-derived cloud names within the service's 24-character limit.
  • Preserve only the exact owned resource in sanitizers; redact unrelated subscription-list identities/tags/descriptions and principal telemetry headers.
  • Retain uncertain mutations, suppress only cleanup 404s and wait for terminal child provisioning before cascade deletion. A completed workload can briefly expose an Accepted child; successful-result assertions are unchanged.
  • Publish seven genuine Node and seven genuine Chromium recordings, refreshed in Sweden Central using the approved plain VHD. Node/browser fixtures are recorded separately, never copied.

Audit Findings Skipped

No fixture fixes deferred. Repeated stress testing was not performed.

Production Code Changed

None. Tests, recorder configuration and asset pointer only; no generated SDK, API, dependency, shared framework or pipeline changes.

Validation Results

Build outcome: LOCAL_BUILD_OK -- 20/20 build tasks successful; TypeScript, formatting and whitespace checks pass. Package lint remains its configured echo skipped.
Tests pass/fail: seven Node integration recordings passed. Six Chromium integration recordings passed in the full live run; the boot passed in an isolated fresh run after fixing the child-provisioning cleanup race. New race regressions pass in both runtimes.
Full playback after final official publication and restore: Node 21/21 and Chromium 21/21, zero failures/errors/skips.
Stress validation: not run.

All fourteen recordings passed raw/nested-JSON auditing: 2,439 entries, 460 scoped catalog/reference IDs, zero privacy/scope violations.
Official assets: js/platformvalidation/arm-platformvalidation_c4bdccbdfa.

Fresh CI 7045610 is running for head a419dd103c. Earlier-head CI results do not validate this update.

Additional Notes

The successful fresh boot's children, CV and managed group are absent; its exact owned empty parent was independently verified and removed. An earlier browser-disconnected run remains conservatively retained because its mutation outcome is uncertain; it is not counted as passing coverage. Pre-existing stalled Poland resources remain untouched.

The approved private image uses existing backend identity access; no storage/RBAC changes. Sweden verification does not imply global rollout. Human review is still required; no review comments posted, threads auto-resolved or merge performed. Separate testing PR after merged #39663.


Metadata:

  • Test-Framework: Vitest
  • Service-Area: PlatformValidation
  • Avg-Confidence: N/A (no scored audit)
  • Findings-Applied: 5
  • Findings-Skipped: 0

Cover catalog discovery, independent cloud validation and plan lifecycles, and approved-image boot execution with ownership-safe cleanup. Publish sanitized Node recording assets and retain the three known catalog scope failures for service/API-owner resolution.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e8e56fca-b29e-4a6c-bb11-f108d5b99032
@github-actions github-actions Bot added the Mgmt This issue is related to a management-plane library. label Oct 29, 2026
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
10 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e8e56fca-b29e-4a6c-bb11-f108d5b99032
@shivangirana20 shivangirana20 changed the title (AI Generated) Test hardening: runs boot execution in PlatformValidation Test hardening: runs boot execution in PlatformValidation Oct 29, 2026
@shivangirana20
shivangirana20 marked this pull request as ready for review October 29, 2026 20:36
Copilot AI balanced review requested due to automatic review settings October 29, 2026 20:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The tests can expose unrelated validation metadata in recordings and SAS credentials in live resources, while polling errors can trigger cleanup during an unresolved operation.

5 open issues
What changed in this PR

Replaces the PlatformValidation SDK’s placeholder test with recorded discovery, lifecycle, and boot-execution coverage. Changes are limited to tests and recording configuration; production SDK code is unchanged.

Changes:

  • Adds seven cases covering provider operations, catalogs, resource lifecycle, and boot results.
  • Adds cleanup safeguards, recorder sanitizers, and live-test environment settings.
  • References Node and browser recordings; reported playback still has three catalog-ID assertion failures.
File Description
sdk/​platformvalidation/​arm-platformvalidation/​test/​public/​utils/​recordedClient.ts Configures credentials, playback variables, sanitizers, and client creation.
sdk/​platformvalidation/​arm-platformvalidation/​test/​public/​sampleTest.spec.ts Removes the placeholder test.
sdk/​platformvalidation/​arm-platformvalidation/​test/​public/​platformValidation.spec.ts Adds discovery, lifecycle, boot-result, and cleanup checks.
sdk/​platformvalidation/​arm-platformvalidation/​sample.env Documents required live-recording settings.
sdk/​platformvalidation/​arm-platformvalidation/​assets.json References the published Node and browser recordings.

Review effort: Balanced

abortSignal: AbortSignal.timeout(timeoutMinutes * 60 * 1000),
});
} finally {
mutationInProgress = !poller.isDone;
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: e8e56fca-b29e-4a6c-bb11-f108d5b99032
Copilot AI balanced review requested due to automatic review settings November 2, 2026 17:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

SAS credentials and unrelated resource metadata can be exposed, cleanup can delete resources during unresolved mutations, and a lifecycle assertion expects the wrong resource ID.

6 open issues
2 previously missed issues

In code that hasn't changed since last review

Medium severity Defer VHD validation until after the CRUD-only branch

sdk/​platformvalidation/​arm-platformvalidation/​test/​public/​platformValidation.spec.ts:105

The cloud-validation-only lifecycle does not create a plan or use a VHD, but it still fails here when PLATFORMVALIDATION_SOURCE_VHD_URI is unset. This prevents running that independent CRUD scenario in environments without an approved image. Read and validate the source, and construct the plan configuration, only after the cloud-validation-only branch has returned.

Medium severity Give each phase its own deadline and reserve cleanup time

sdk/​platformvalidation/​arm-platformvalidation/​test/​public/​platformValidation.spec.ts:127

[P2] This signal starts its 30-minute countdown before any resource is created and is reused for every later operation. For example, 20 minutes spent creating the CV and plan leaves only 10 minutes for the run, despite its 20-minute poller timeout and subsequent 30-minute status loop; a valid slow run is aborted and retained. Give each phase its own deadline and reserve sufficient time for cleanup in the overall test timeout.

Review effort: Balanced

Use credential-free fixtures and valid UUID-derived names, preserve ownership-scoped sanitization, and wait for terminal child provisioning before cleanup. Refresh genuine Node/browser recordings in Sweden Central.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e8e56fca-b29e-4a6c-bb11-f108d5b99032
Copilot AI balanced review requested due to automatic review settings November 6, 2026 03:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

const result = await read();
if (
["Succeeded", "Failed", "Canceled"].includes(result.properties?.provisioningState ?? "") &&
["Completed", "Failed", "TimedOut", "Canceled"].includes(result.properties?.status ?? "")

This branch has not been deployed

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

Labels

Mgmt This issue is related to a management-plane library.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants