test: establish dashboard plugin regression baseline - #372
Merged
nithyatsu merged 30 commits intoSep 16, 2026
Merged
Conversation
Records the current regression baseline (31 of 71 source files have a colocated test; 15 unit test files contain a single case) and lays out a nine-phase plan to build coverage for existing functionality before the plugin rearchitecture, then extend it to the plugin contract, host integration, and graph parity with the ai-extensions graph elements. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: nicolejms <nicolej@microsoft.com>
… design Adds the companion design doc (moved from ai-extensions) and reworks the test plan to match the architecture it decides. Graph testing is restructured around whether a test is allowed to break, since the consolidation replaces the node objects, edge objects, layout inputs, and markup wholesale: - Tier A invariants and Tier B observable behavior must not change, and are the regression net. - Tier C diffs a normalized graph record against a baseline frozen before extraction. Every difference must map to an entry in a reviewed expected-change manifest, which is how an intended change is separated from an unintended one. - Tier E implementation unit tests are deleted with the code they describe rather than migrated. - GU-20 asserts the suite fails without the real renderer, so it cannot pass against a stub the way ApplicationTab does today. Also reorders the phases so the real-renderer baseline is frozen before any extraction, replaces the cross-repo parity approach (the design rejects duplicated implementations), retires rad-components as an implementation owner, and adds requirements for connections, error states, package boundaries, the installed artifact, the React matrix, and the supported consumer pin. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: nicolejms <nicolej@microsoft.com>
The layer table named runners without recording who owns the Jest config or what the tooling implies, which left two things unstated that the plan depends on. - backstage-cli repo test supplies the Jest config; there is no jest.config.js and no jest key in any workspace today. Coverage floors are therefore per-workspace overrides the CLI merges, not a standalone config. - jest-canvas-mock exists because React Flow calls canvas APIs jsdom lacks, so a stubbed canvas can satisfy a render assertion without laying anything out. That is why Tier B and Tier C run in Chromium. - ai-extensions uses Vitest and dashboard uses Jest, so graph-react crosses a runner, module-format, and transform boundary when consumed here. Upstream passing tests are not evidence for this host, which is what IA-01-IA-08 and CP-01-CP-05 cover. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: nicolejms <nicolej@microsoft.com>
Keeping Jest was stated without a reason, which invites the question in review and gives no basis for revisiting it later. Backstage offers no supported Vitest path, so adopting Vitest means leaving the Backstage build system for tests and re-aligning per-workspace config on every CLI upgrade. The stronger objection is sequencing: Phases 0-2 freeze current behavior so extraction can be diffed against it, and changing runners inside that window makes every failure ambiguous between an extraction fault and a migration artifact. The migration also does not address the motivating risk, since the graph tiers run in Playwright, which is runner-agnostic. Also notes that the runner boundary with ai-extensions follows from ownership rather than tooling preference, so converting dashboard would not remove it, and that testing a dependency under its author's runner is weaker evidence because it can hide packaging and interop faults. Recorded as open decision 5 rather than a rejection, to be taken after Phase 4 against a frozen baseline. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: nicolejms <nicolej@microsoft.com>
The phase table did not say where each phase is executed or what it waits on, which reads as though the packaging phase belongs to ai-extensions. It does not. Adds a repository column and states that every phase runs in dashboard, with the column recording dependencies rather than location. Only Phase 4 is blocked on ai-extensions, since that is where dashboard swaps its implementations for the published packages. Phases 6 and 8 span both repositories because the consumer-pin gate is defined upstream while the pin, the journey it invokes, and the release checks live here. Phase 3 is called out explicitly: the design assigns core and graph-react to ai-extensions and the Backstage plugin to dashboard, so hardening plugins/plugin-radius into a published package touches no shared code and waits on no upstream release. It is the only pre-extraction stream not gated on the graph consolidation. Also records the npm scope as open decision 6. Phase 3 asserts the plugin's published name and the installed-artifact and consumer-pin requirements reference all three names, so the scope should be confirmed before those assertions are written. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: nicolejms <nicolej@microsoft.com>
Phase 0 records current behavior and makes it enforceable; phase 3 pins the plugin as a published contract. Executing them corrected several assumptions the plan had been written on. Coverage floors are measured, not estimated, and are enforced. The non-obvious part is where they live: CI runs `backstage-cli repo test`, which runs each workspace as a Jest project, and Jest refuses `coverageThreshold` in a project config. It warns and continues, so a per-workspace floor passes review, passes CI, and gates nothing -- while still working when a single workspace is run alone, so a local check agrees with an expectation CI does not share. Floors therefore move to the root config as path groups. Because a path group removes its files from `global`, a `global` entry would then measure nothing and fail at 0%, so there is none; PU-23 closes the resulting hole by failing when a workspace has neither a floor nor a recorded exemption. Enforcement is verified by a negative test rather than assumed. Phase 3 replaces the single "plugin is defined" assertion with contract and packaging tests, and records three blockers as KNOWN-DEFECT rather than fixing them: the package is private, it depends on a workspace-only range no external consumer can resolve, and `radiusApiRef` is not exported from the entry point, so a host cannot override the API it is supposed to configure. Two claims in the plan were wrong and are corrected. The license situation is worse than described: the repository root declares no license at all, and `rad-components` declares ISC while not being private, making the one publishable package the one that disagrees with the repository. The resourceId duplication was the inverse of what was recorded -- all eleven consumers import from `rad-components`, while the plugin's copy had no importers and 0% coverage. It is deleted, and its test moved to the package it always imported from. Also records a pre-existing flake found while measuring: five page suites exceed Jest's 5s default under parallel load but pass in isolation. It is left unfixed deliberately, because changing the harness during the window in which phases 0-2 freeze behavior is the thing this plan forbids elsewhere. Signed-off-by: nicolejms <nicolej@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Establishes the graph-model test seam and the first tier of the graph test taxonomy, so the AppGraph rewrite and the later extraction of graph rendering into a shared package can be verified rather than eyeballed. Adds: - packages/rad-components/src/__fixtures__/graph/ - the 14 Appendix E fixtures (empty, single-node, container-to-database, gateway-inbound, multi-tier, unparseable-connection, missing-target, self-reference, managed-cluster, deploy-status-matrix, unknown-type, duplicate-ids, both-namespaces, large-fan-out). Shared JSON modules, deep-cloned per use because the graph builder mutates its input. - packages/rad-components/src/graphModel.ts - buildGraphModel and buildLayoutedGraphModel. Tier A asserts against this adapter rather than against initialNodes directly, so the tests survive extraction: phase 4 repoints this one file at the shared package and the invariants are unchanged. - packages/rad-components/src/__test__/graphInvariants.test.ts - Tier A invariants GU-01 through GU-10 plus node identity, edge direction, and degenerate input suites. Changes: - AppGraph.tsx: export initialNodes and getLayoutedElements, and give getLayoutedElements an explicit return type so exporting it does not leak reactflow's internalsSymbol across the package boundary (TS4058). No behaviour change. - package.json: raise the packages/rad-components coverage floors to the newly measured levels - statements 86, branches 81, functions 80, lines 85. Tier A moved that workspace from 81.33/63.64 to 86.52/81.82 statements/branches. Writing the invariants surfaced four real defects in the graph builder. All are recorded as KNOWN-DEFECT assertions describing current behaviour and none are fixed here, per the baseline-freeze rule: - initialNodes mutates its input, rewriting connection.direction in place. - An unparseable connection id is not skipped. The parse result only gates the direction rewrite; the edge loop still runs and emits a dangling edge. - A connection to a resource absent from the payload emits the same dangling edge. - A self-referential connection emits a self-loop, and duplicate resource ids emit duplicate node ids. GU-08 covers the module-level Dagre graph leak. Detecting it needs jest.isolateModules: the leaked state is a module-level binding, so the first layout in a file pollutes every later one and a naive A-then-B versus B-alone comparison passes as a false negative. Verified: yarn tsc, yarn lint:all and yarn format:check clean; yarn test:all --maxWorkers=2 green at 34 suites / 260 tests with the raised floors enforced. Signed-off-by: nicolejms <nicolej@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
RecipeListPage and ResourceListPage were both at 0% statement coverage, the two largest untested pages after ResourceTypeDetailPage. Both new suites assert the error path, which no page test in the repository did before. RecipeListPage (RE-01 through RE-08). The aggregation already had unit tests, but nothing exercised the asynchronous half of the feature: recipe pack ids live on the environment and each pack is fetched separately. Covers the fetch fan-out, the Promise.allSettled tolerance of an unreachable pack, the de-duplication of a pack referenced by two environments, the legacy inline recipes path, and switching environments through the selector. ResourceListPage (RL-01 through RL-04). The page is a thin shell over ResourceTable, but it is the only place the table renders without a resource type, which selects the Type/Application/Environment/Status column set rather than the environment one. That column set had no test. Both error-path assertions had to match all occurrences rather than one: ResponseErrorPanel renders the message twice, in the summary heading and again in the expanded detail list, so getByText fails with "found multiple elements" for a reason unrelated to the behaviour under test. Recorded in the plan as a trap, since the obvious assertion looks correct. plugins/plugin-radius moves from 58.42% to 61.22% statements and 40.80% to 46.23% functions. Floors raised to 61 / 33 / 46 / 60 accordingly, and the plan's progression table and Appendix G enforced-floor table updated to match. Verified: yarn tsc, yarn lint:all and yarn format:check clean; yarn test:all --maxWorkers=2 green at 36 suites / 272 tests with the raised floors enforced. Signed-off-by: nicolejms <nicolej@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Almost the entire test corpus is written against the legacy Applications.Core namespace: `Applications.` appears in 40 files, including every graph fixture added in phase 2 and 57 occurrences in api.test.ts, against 15 files for `Radius.`. The dashboard's own UI already leans the other way, since ResourceTypesTable excludes 'Applications.' from the resource types page. Records one correction to the premise. Applications.Core is not formally deprecated: no announcement, issue or release note declares it so, no removal release is named, the Applications.* providers are still registered by default, the Dapr integration still requires them and has no Radius.* equivalent, and the Radius.* types remain preview-gated. There is also no migration guide. The work is therefore driven by the baseline freeze rather than by a deadline - freezing a baseline that describes only the legacy model bakes legacy assumptions into the thing meant to detect regressions. Also records that this is not a namespace rename. Radius.Core holds only five first-class types; everything else became a user-defined resource type spread across six further namespaces, and several legacy types have no successor. Includes the mapping table, with inferred entries marked as such. Captures what the fixtures must actually change: - Resource ids. Type names may contain digits (Radius.Data/neo4jDatabases), namespaces may too, and globally-scoped recipe packs produce ids with no resourceGroups segment at all. All three break the current parser, which is the same parser already failing on '.' and '_'. - API versions are per-type, not per-namespace, so no fixture may hardcode one. - Environments drop properties.compute in favour of providers.kubernetes.namespace, drop recipes in favour of recipePacks, and change the shape of providers rather than just its contents. - getGraph gained a required connections[].kind enum plus icons and iconHash, and the upstream wire-change note names the dashboard as an affected consumer. Our graph model has no concept of edge kind, so this is a feature gap. Work items NS-01 through NS-10. Two are defects found while writing this up: sampledata.ts declares type 'Applications.Core/container' singular while its own id says containers and AppGraph compares against the plural, so the container layout branch has never been exercised by the sample data; and AppGraph's two hardcoded namespace couplings silently stop applying under the new model, since containers and gateways become Radius.Compute/containers and Radius.Compute/routes. Sequenced deliberately rather than by dependency. It must not run during phases 1 and 2, which freeze current behaviour, nor be folded into phase 4, since either would change fixtures and implementation together and leave no diff attributable. The proposed slot is after phase 4's record diff is green, as a Tier C expected-change event. Carried as open decision 8, because the alternative of moving before phase 1 and never freezing a legacy baseline is cheaper if maintainers expect Applications.* to be unsupported sooner than this plan assumes, and deciding late is the expensive option. Signed-off-by: nicolejms <nicolej@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Nine issues were filed upstream in radius-project/dashboard for the defects this plan characterizes. Without a link back, a KNOWN-DEFECT assertion is indistinguishable from a blessing: it records what the code does today, and nothing in the test says whether anyone intends to change that. Add a Tracked defects registry mapping each issue to the assertions that pin it, so the relationship is visible from the plan rather than only from the issue tracker. Two entries are recorded as not yet pinned rather than quietly omitted. radius-project#352 is pinnable whenever, since parseResourceId is stable. radius-project#356 is not: the cluster-selection divergence is observable today and stops being observable once the graph request moves, so it has to be characterized during Phase 2 or it cannot afterwards be shown to have been preserved or fixed. That deadline is the reason the distinction is worth writing down. The pre-existing resourceId test is explicitly not counted as pinning radius-project#352, because it predates this plan and carries no ids. Signed-off-by: nicolejms <nicolej@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Adds direct tests for ten of the thirteen plugin-radius components that had none, and deepens the ResourceTypeDetailPage suite from a smoke test to a behavioral one. New suites, using the per-file test-id prefixes now documented in Appendix B: - ApplicationListInfoCard (AC-01-AC-08) - EnvironmentListInfoCard (EC-01-EC-08) - EnvironmentResourcesTab (EV-01-EV-03) - OverviewTab (OT-01-OT-07) - DetailsTab (DT-01-DT-03) - ApplicationResourcesTab (AR-01-AR-03) - ResourceLayout (LY-01-LY-04) ResourceTypeDetailPage is rewritten as RT-01-RT-27, covering tab routing, the recursive schema walker, and the read-only property filters. The rad-components resourceId suite is tagged RU-01/RU-02 so the parser defects are pinned at the level they occur. Three defects were found and filed upstream rather than fixed here, so the tests record current behavior with KNOWN-DEFECT annotations: - radius-project#361 placeholder markdown describing Applications.Core/containers renders for any resource type with no description - radius-project#362 ResourceLayout renders the literal string "undefined/undefined: undefined" when mounted off-route - radius-project#363 the Output Properties tab inverts its read-only filter for nested properties, hiding read-only children and showing writable ones A follow-up comment on radius-project#352 records that an unparseable resource id surfaces as a misleading scrollWidth error, because ResourceLink throws during render and MaterialTable then dereferences a null ref. plugin-radius coverage floors rise from 61/33/46/60 to 69/46/58/68, measured, not aspirational. Full suite: 43 suites, 331 cases, tsc and lint clean. Signed-off-by: nicolejms <nicolej@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Assert graph topology, harden coverage-policy guards, exercise real component routes, and correct packaging and phase-completion claims. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: nicolejms <nicolej@microsoft.com>
Phase 1 requires every shipped page, table, tab, card, and domain rule to have a real test before the plugin rearchitecture starts. Layer 1 closed ten of the thirteen untested plugin-radius components. This closes the rest. plugin-radius: RecipeTable (RK-01--RK-10), routes.ts (RO-01--RO-07), features.ts (FF-01--FF-05), and resources/resource.ts (RS-01--RS-14). resource.ts emits no JavaScript, so RS is compile-time characterization: each @ts-expect-error asserts a shape the model must still reject and fails yarn tsc if the model is widened. packages/app: Root (RR), HomePage (HP), LearnCard (LC), CommunityCard (CC), SupportCard (SC), and apis.ts (AP). The workspace read 75% statements with 0% branches and 0% functions, which is the signature of coverage produced by module loading rather than by testing. It now reads 93.51/100.00/83.33/92.86. index.tsx is left alone deliberately: it boots the real app. packages/backend: BK-01--BK-06 for the entry point, taking it from 0% to 100%, which let its coverage exemption be deleted from coveragePolicy.test.ts and replaced by a measured floor. The exemption list is now empty and PU-31 asserts it stays empty, so a future exemption has to be argued for rather than accumulated. Covering the entry point needed a documented jest.transform override in packages/backend/package.json: the CLI compiles backend-plugin packages with SWC module.ignoreDynamic, leaving import() native, and Jest's CJS runtime then refuses to execute it. The Sucrase transform also runs but its getCacheKey ignores Jest's instrument flag, so a cached uninstrumented compile is reused and the file silently reports 0% -- SWC is the correct fix. Floors raised in the root package.json, in the same commit as the tests that earned them: plugin-radius 69/46/58/68 to 70/50/61/70, packages/app 75/-/-/78 to 93/100/83/92, and a new packages/backend group at 100/-/100/100. Two defects found and characterized rather than fixed, per the Phase 1 rule: radius-project#364, where "Join us on Discord" carries to="" and so navigates to the dashboard home page (CC-05, CC-06), and radius-project#365, where Resource.systemData is required and typed Record<string, never> so every fixture must cast around it (RS-14). Both are recorded in the plan's Tracked defects table. Plan updated: Appendix B gains the eleven new prefixes and PU-31, Appendix F records every closed file and why index.tsx and setupTests.ts stay open, Appendix G carries the new enforced floors with the backend exempt row gone, and the coverage-progression and phase tables are re-measured. Phase 1 is marked done. 53 suites / 411 tests, up from 43/331. yarn tsc, yarn lint:all, and yarn format:check are clean. Signed-off-by: nicolejms <nicolej@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
radius-project#366 is a harness defect with no KNOWN-DEFECT assertion pinning it, so it is recorded the way radius-project#360 is. Two things make it unlike every other row in the table, and the row has to say both or it misleads the next reader. It cannot be fixed here. The defective getCacheKey is in the published @backstage/cli-module-test-jest; @backstage/cli/config/jestSucraseTransform.js is a 27-line re-export shim, so the fix has to land in backstage/backstage. This repository's exposure is currently zero. The CLI builds its transform map entirely from jestSwcTransform, nothing here selects Sucrase, and the packages/backend override added in Phase 1 uses SWC. The earlier Appendix F wording implied a live hazard; it is corrected to say the trap is armed by a future edit rather than by existing code. It is tracked anyway because the edit that arms it is one a maintainer is actively likely to make: Sucrase lowers import() to require unconditionally, which is exactly what a Backstage backend entry point needs to be testable under Jest's CommonJS runtime, so it is the first override that appears to work. It then reports 0% on a file whose tests pass, which under the Phase 6 merge gate surfaces as a threshold failure naming a path group that is green -- where the obvious response of lowering the floor is precisely wrong. Documentation only. No test, source, or coverage-floor changes. Signed-off-by: nicolejms <nicolej@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The rubber-duck review found that Phase 1 was marked done while the Radius backend plugin still had only its one-case health smoke test. Implement the BE-01--BE-05 contract promised by the plan: health and 404 behavior, plugin id and service dependencies, router mounting and initialization logging, and startup failure propagation. The new coverage moves plugin-radius-backend from 62.50/n-a/50.00/71.43 to 93.75/n-a/100/100, and the root floors rise with it. Harden the rest of the Phase 1 evidence found by the same review: - copy all six Backstage Jest transform entries into packages/backend while dropping module.ignoreDynamic only from the JS/TS entries that need it; - make backend entry-point mocks resolve real declared dependencies instead of virtual modules; - replace broad @ts-expect-error checks in the Resource model suite with exact property-type and optional-key assertions, including explicit lifecycle guidance for the radius-project#365 defect pin; - correct RO-07's three-route description; - add PU-35, which prevents a workspace manifest or supported jest.config.js/ts override from selecting the Sucrase transform whose cache key ignores Jest's instrument flag; - preserve the lower layer's PU-32--PU-34 sensitivity tests, graph invariants, RT-28--RT-30 routes and request contracts, parent-discriminating AR/EV rows, and Phase 3 in-progress status while rebasing onto 844d60d; - reconcile Appendix F, Phase 1 completion evidence, coverage progression, floors, defect guidance, and the backend-plugin index-barrel description; - record a successful default-worker `yarn test:all` recheck for radius-project#360 rather than leaving a completed phase pointing at an unexamined decision. Measured on the rebased combined tree: 53 suites / 427 tests. Coverage is 72.70/54.69/64.32/72.77 for plugin-radius, 93.75/n-a/100/100 for plugin-radius-backend, 86.52/81.82/80.95/85.00 for rad-components, 93.51/100/83.33/92.86 for packages/app, and 100/n-a/100/100 for packages/backend. Floors rise to the rounded-down measured values earned by this tree. Validation: yarn tsc, yarn lint:all, yarn format:check; 7 rebase-sensitive suites / 71 tests; default-worker yarn test:all at 53/427; and full coverage at 53/427 with --maxWorkers=2 and all raised floors enforced. Signed-off-by: nicolejms <nicolej@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Add semantic graph records, deterministic browser journeys, and connection/error characterization before plugin extraction. Raise measured workspace coverage floors and record the completed Phase 2 evidence. Signed-off-by: nicolejms <nicolej@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Align the test annotations and plan ledger with the issue numbers assigned when the Phase 2 defects were filed. Signed-off-by: nicolejms <nicolej@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Publish the frontend plugin under its approved package name, expose its API contract, and add executable factory, lazy-route, packed-artifact, declaration-consumer, and import-boundary gates. Signed-off-by: nicolejms <nicolej@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: nicolejms <nicolej@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: nicolejms <nicolej@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: nicolejms <nicolej@microsoft.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The graph-record update gate can miss or reject legitimate changes, and parts of the semantic and lifecycle baseline can produce false passes.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Establishes a regression baseline for the planned Radius graph and Backstage plugin extraction.
Changes:
- Adds extensive component, API, graph, packaging, and browser tests.
- Introduces semantic graph fixtures, records, and invariants.
- Enforces workspace coverage thresholds and package contracts.
File summaries
| File | Description |
|---|---|
plugins/plugin-radius/src/routes.test.ts |
Tests route contracts. |
plugins/plugin-radius/src/resources/resourceId.ts |
Removes duplicate parser. |
plugins/plugin-radius/src/resources/resourceId.test.ts |
Removes superseded tests. |
plugins/plugin-radius/src/resources/resource.test.ts |
Tests resource types. |
plugins/plugin-radius/src/plugin.ts |
Exports API type. |
plugins/plugin-radius/src/plugin.test.ts |
Tests plugin contract. |
plugins/plugin-radius/src/packagingArtifact.test.ts |
Qualifies packed artifacts. |
plugins/plugin-radius/src/packaging.test.ts |
Tests package metadata. |
plugins/plugin-radius/src/index.ts |
Publishes API exports. |
plugins/plugin-radius/src/importBoundaries.test.ts |
Enforces import boundaries. |
plugins/plugin-radius/src/features.test.ts |
Tests feature flags. |
plugins/plugin-radius/src/coveragePolicy.test.ts |
Validates coverage policy. |
plugins/plugin-radius/src/components/resources/ResourceListPage.test.tsx |
Tests resource listing. |
plugins/plugin-radius/src/components/resources/ResourceLayout.test.tsx |
Tests resource layout. |
plugins/plugin-radius/src/components/resources/OverviewTab.test.tsx |
Tests overview behavior. |
plugins/plugin-radius/src/components/resources/DetailsTab.test.tsx |
Tests JSON details. |
plugins/plugin-radius/src/components/resources/ApplicationTab.test.tsx |
Expands graph tests. |
plugins/plugin-radius/src/components/resources/ApplicationResourcesTab.test.tsx |
Tests application filtering. |
plugins/plugin-radius/src/components/recipes/RecipeTable.test.tsx |
Tests recipe table. |
plugins/plugin-radius/src/components/recipes/RecipeListPage.test.tsx |
Tests recipe loading. |
plugins/plugin-radius/src/components/environments/EnvironmentResourcesTab.test.tsx |
Tests environment filtering. |
plugins/plugin-radius/src/components/environments/EnvironmentListPage.test.tsx |
Expands environment tests. |
plugins/plugin-radius/src/components/environments/EnvironmentListInfoCard.test.tsx |
Tests environment card. |
plugins/plugin-radius/src/components/applications/ApplicationListInfoCard.test.tsx |
Tests application card. |
plugins/plugin-radius/src/api/api.test.ts |
Expands API failure coverage. |
plugins/plugin-radius/package.json |
Adds plugin metadata. |
plugins/plugin-radius/index.ts |
Adds package exports. |
plugins/plugin-radius-backend/src/service/router.test.ts |
Tests backend registration. |
playwright.config.ts |
Adds Storybook server configuration. |
packages/rad-components/src/graphRecord.ts |
Adds semantic graph records. |
packages/rad-components/src/graphModel.ts |
Adds graph-model adapter. |
packages/rad-components/src/components/appgraph/AppGraph.tsx |
Exposes graph helpers. |
packages/rad-components/src/components/appgraph/__test__/AppGraph.test.tsx |
Tests layout failure. |
packages/rad-components/src/components/appgraph/__docs__/AppGraph.stories.tsx |
Adds graph scenarios. |
packages/rad-components/src/__test__/resourceId.test.ts |
Expands parser coverage. |
packages/rad-components/src/__test__/graphRecords.test.ts |
Tests graph records. |
packages/rad-components/src/__test__/graphInvariants.test.ts |
Defines graph invariants. |
packages/rad-components/src/__fixtures__/graph/unparseable-connection.json |
Adds malformed-edge fixture. |
packages/rad-components/src/__fixtures__/graph/unknown-type.json |
Adds unknown-type fixture. |
packages/rad-components/src/__fixtures__/graph/single-node.json |
Adds single-node fixture. |
packages/rad-components/src/__fixtures__/graph/self-reference.json |
Adds self-edge fixture. |
packages/rad-components/src/__fixtures__/graph/multi-tier.json |
Adds multi-tier fixture. |
packages/rad-components/src/__fixtures__/graph/missing-target.json |
Adds dangling-edge fixture. |
packages/rad-components/src/__fixtures__/graph/managed-cluster.json |
Adds managed-resource fixture. |
packages/rad-components/src/__fixtures__/graph/large-fan-out.json |
Adds fan-out fixture. |
packages/rad-components/src/__fixtures__/graph/gateway-inbound.json |
Adds gateway fixture. |
packages/rad-components/src/__fixtures__/graph/empty.json |
Adds empty fixture. |
packages/rad-components/src/__fixtures__/graph/duplicate-ids.json |
Adds duplicate-ID fixture. |
packages/rad-components/src/__fixtures__/graph/deploy-status-matrix.json |
Adds status fixture. |
packages/rad-components/src/__fixtures__/graph/container-to-database.json |
Adds dependency fixture. |
packages/rad-components/src/__fixtures__/graph/both-namespaces.json |
Adds namespace fixture. |
packages/rad-components/src/__fixtures__/graph-records/unparseable-connection.json |
Records malformed edge. |
packages/rad-components/src/__fixtures__/graph-records/unknown-type.json |
Records unknown type. |
packages/rad-components/src/__fixtures__/graph-records/single-node.json |
Records single node. |
packages/rad-components/src/__fixtures__/graph-records/self-reference.json |
Records self-edge. |
packages/rad-components/src/__fixtures__/graph-records/multi-tier.json |
Records multi-tier graph. |
packages/rad-components/src/__fixtures__/graph-records/missing-target.json |
Records dangling edge. |
packages/rad-components/src/__fixtures__/graph-records/managed-cluster.json |
Records managed resource. |
packages/rad-components/src/__fixtures__/graph-records/large-fan-out.json |
Records fan-out graph. |
packages/rad-components/src/__fixtures__/graph-records/gateway-inbound.json |
Records gateway graph. |
packages/rad-components/src/__fixtures__/graph-records/empty.json |
Records empty graph. |
packages/rad-components/src/__fixtures__/graph-records/duplicate-ids.json |
Records duplicate IDs. |
packages/rad-components/src/__fixtures__/graph-records/deploy-status-matrix.json |
Records status graph. |
packages/rad-components/src/__fixtures__/graph-records/container-to-database.json |
Records dependency graph. |
packages/rad-components/src/__fixtures__/graph-records/both-namespaces.json |
Records namespace graph. |
packages/rad-components/src/__fixtures__/graph-expected-changes.md |
Defines change manifest. |
packages/rad-components/package.json |
Adds build dependency. |
packages/rad-components/jest.config.json |
Refines coverage exclusions. |
packages/rad-components/e2e-tests/appGraph.test.ts |
Adds renderer journeys. |
packages/backend/src/index.test.ts |
Tests backend entry point. |
packages/backend/package.json |
Configures Jest transforms. |
packages/app/src/components/Root/Root.test.tsx |
Tests host navigation. |
packages/app/src/components/home/SupportCard.test.tsx |
Tests support links. |
packages/app/src/components/home/LearnCard.test.tsx |
Tests learning links. |
packages/app/src/components/home/HomePage.test.tsx |
Tests home composition. |
packages/app/src/components/home/CommunityCard.test.tsx |
Tests community links. |
packages/app/src/apis.test.ts |
Tests API factories. |
packages/app/package.json |
Adds test dependency. |
packages/app/e2e-tests/radiusGraph.test.ts |
Adds host graph journeys. |
packages/app/e2e-tests/fixtures/multi-tier.json |
Adds browser fixture. |
package.json |
Enforces coverage floors. |
Review details
- Files reviewed: 83/85 changed files
- Comments generated: 4
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: nicolejms <nicolej@microsoft.com>
Restore the repo-wide esbuild resolution pin that was dropped while iterating on the packaging tests. Removing it admitted esbuild 0.28.2 plus 26 platform packages, and resolving those through the local proxy registry recorded internal __archiveUrl fetch locations into yarn.lock. Yarn treats __archiveUrl as a hard fetch location rather than a hint, so those entries would have broken installs for anyone outside that network. The lockfile now carries zero of them and differs from the merge base only by the two intended devDependency additions. The real build-and-pack artifact suite passes with the pin restored, which shows the removal was never required. Also address the non-blocking review points: - packaging.test.ts reads the source manifests with a retry-until-stable loop so a concurrent prepack window cannot be observed mid-mutation, and documents which fields prepack rewrites. - graphRecords.test.ts skips the GU-21 cases in update mode instead of reporting them as passing without assertions, and GU-22c derives its expected old value from the baseline rather than hardcoding it. - playwright.config.ts lets PLAYWRIGHT_DISABLE_WEBSERVER hand over both the app and the Storybook host instead of only the app. - plugins/plugin-radius/index.ts drops two re-exports the src barrel already provides. - Both backend coverage path groups gain a measured 100% branch floor. - Appendix G records that the backend figure is measured under the SWC transform, so it does not verify production ESM module loading. Signed-off-by: nicolejms <nicolej@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Storybook E2E startup is broken for built-container CI, and graph-record safeguards have bypass and false-negative paths.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 84/86 changed files
- Comments generated: 4
- Review effort level: Balanced
The previous commit folded the Storybook host into the PLAYWRIGHT_DISABLE_WEBSERVER guard on the assumption that the flag means "all servers are externally managed". It does not. The release workflow sets it together with PLAYWRIGHT_URL so the Backstage app is served by the freshly built container, but that image contains the app alone, so nothing serves Storybook. Gating both entries made the rad-components browser suite fail in CI with ERR_CONNECTION_REFUSED on port 6006. Restore the original gating, which drops only the app entry, and replace the comment that caused the misreading with one that states why the two entries are deliberately gated differently. Signed-off-by: nicolejms <nicolej@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
nithyatsu
reviewed
Sep 15, 2026
nithyatsu
left a comment
nithyatsu
left a comment
Contributor
There was a problem hiding this comment.
Went through this properly. The core idea is right, and I want it to land: the two-tier split — Tier A invariants written against a deliberate buildGraphModel seam, plus the fixture-driven semantic records — is the right shape for a pre-migration baseline. I spot-checked the KNOWN-DEFECT pins (#35, #41, #89, #353, #357, #366, #368) and they all point at real open issues, which I appreciate.
Most of my concerns are in the non-graph parts, and they're inline. The three I'd call blocking are the pack-inside-Jest race in packagingArtifact.test.ts, its use of .copilot-tracking/, and the no-op getResourceNodeSemantics shipping in the published package.
The one that matters most for what this baseline is for: it's strongest exactly where the migration preserves behaviour, and weakest exactly where it doesn't. The Tier A invariants will survive the move intact — that's the valuable part. But the quantized positions won't (every one of the fourteen changes at once on any layout change, so the whole suite gets blanket-approved in a single commit), and the icon/statusBadge pins encode today's absence as the target. Details on both inline on graphRecord.ts.
Nothing here is a criticism of the approach — mostly I want the records to be pinning things that still mean something on the other side of the extraction.
Address four review findings on the graph regression baseline. The known-defect tracker compared declared record fields, so any descendant change cleared the defect: populating nodes.0.icon would have reported the duplicate-id defect as repaired while the duplicate ids were still there. Each entry now carries an isPresent predicate that reads the violated invariant off the record - a dangling edge endpoint, a self-edge, duplicate ids, absent icons, absent status badges - so only an actual repair retires an entry. GU-23b pins every predicate against both a healthy and a violating record so none can decay into a constant, and GU-23a proves the icon case that the old tracker got wrong. The expected-change manifest was also bypassable. GU-21 and GU-22 both compare generated records against the committed ones, so a change that edits a record file and the renderer together satisfied both without consulting the manifest. GU-25 diffs the committed records against the base branch, where the previous baseline still exists, and requires a manifest entry for every difference. It resolves the base from GRAPH_RECORD_BASE_REF then origin/main and fails rather than skipping when no base is reachable, so the build workflow now checks out full history. A record absent at the base is a new fixture, not a mutated baseline, and needs no entry. Verified by tampering with a committed record and confirming GU-25 rejects it. The import-boundary check searched for '/src/' and '/private/', which missed a specifier ending at the private segment such as '@radapp.io/rad-components/src'. It now matches whole path segments after the package name, and PB-02b pins the exact-suffix forms along with the public entry points that must stay allowed. The Playwright finding was fixed in 314f230. Full suite: 56 suites / 477 tests, coverage floors unchanged. Signed-off-by: nicolejms <nicolej@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The previous commit's edit swallowed the "- name: Lint" line, leaving the install step with two run keys. That made build.yaml unparseable, so the whole workflow failed before any job started and GitHub reported the run under the file path instead of its name. Restore the step and verify by parsing the file: the workflow is named Build again and both jobs carry their full step lists, with the only differences from main being the intended fetch-depth and GRAPH_RECORD_BASE_REF additions. Signed-off-by: nicolejms <nicolej@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…m records Maintainer review asked for the real fix behind GU-08 rather than a longer-lived characterization pin. `getLayoutedElements` held a module-level `Dagre.graphlib.Graph`, so every layout accumulated the previous application's nodes and edges. It now constructs its own graph per call, GU-08 asserts equality between the sequential and standalone layouts in both orderings, and the `jest.isolateModules` machinery that existed only to observe the leak is gone. Closes radius-project#355. Graph records no longer carry positions. Coordinates come from Dagre, not from the dashboard, so recording them pinned a third-party algorithm's output as if it were dashboard semantics. The properties that matter are asserted directly in Tier A (GU-08, GU-09); GU-21c pins their absence from the corpus. Unpopulated icon and status fields are recorded as an explicit `not-yet-populated` sentinel instead of `null`, because `null` is a value a renderer could legitimately produce and would be indistinguishable from one that cleared them. The fourteen baselines were regenerated through the manifest gate, which correctly refused the schema change until every field difference was declared; the manifest is emptied again per GU-24. Signed-off-by: nicolejms <nicolej@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…w debt Address maintainer review on radius-project#372. - Gate the build-and-pack suite behind PACKAGE_QUALIFICATION so the unit run never shells out to a build; add scripts/test-package.js, a test:package script, and a CI step, and pin all three with PP-01 so the suite cannot silently stop running. - Stage packed artifacts under os.tmpdir() instead of .copilot-tracking, and drop the Atomics.wait spin-lock. - Import resource-node semantics from their defining module rather than the component barrel. - Split the component browser suite into playwright.components.config.ts with its own Storybook web server, and move the three test-only stories into AppGraphHarness.stories.tsx so they stay out of the docs site. - Restore mocks after each AppGraph case. - Replace the package-metadata pins with a release checklist, drop the exact-keys feature assertion, and cut the e2e leak-detector self-test. Signed-off-by: nicolejms <nicolej@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- Explain why the Tier C corpus stays on Applications.Core/* until the AppGraph type comparisons become namespace-aware, and track that as radius-project#373. - Record why the repo-policy suites cannot move to the repository root: backstage-cli repo test only discovers tests inside workspaces and exits 0 when it finds none, so a root location passes vacuously. Signed-off-by: nicolejms <nicolej@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
nithyatsu
approved these changes
Sep 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Establish the pre-extraction regression baseline for adopting shared Radius graph components and packaging the dashboard as a Backstage plugin.
The plugin remains
@internal/plugin-radiusand private. Shared-package extraction, final two-host installation, and release decisions remain deferred. Known product defects are characterized, not all fixed.Validation
yarn test:package): 5/5.rad-components95.89%,packages/app93.51%,packages/backend100%,plugin-radius73.89%. Enforced floors unchanged and holding.No packages published.