test(bulk-import): cover the NFS wiring, not just the translations module - #4484
Open
gustavolira wants to merge 1 commit into
Open
test(bulk-import): cover the NFS wiring, not just the translations module#4484gustavolira wants to merge 1 commit into
gustavolira wants to merge 1 commit into
Conversation
…dule nfsExports.test.ts asserted only that the translations module exists and is exported as default. That module is a separate FrontendModule with pluginId 'app', so both assertions hold against a bulk-import plugin whose own `extensions` array is empty — the page and the API disappear from the app with no error, no console warning, exit 0. Adds what the app resolves off the extensions: the page's routePath and title, that the page and the API are registered on the plugin, and that both route refs survive. Mutation-verified, six for six: page dropped from extensions -> 2 failed api dropped from extensions -> 1 failed path '/bulk-import' -> 1 failed title 'Bulk import' -> 1 failed routeRef removed -> 1 failed tasks subRouteRef removed -> 1 failed One thing worth knowing before this pattern is copied further. The equivalent test in adoption-insights asserts `toBe(rootRouteRef)` on the route ref, and that passes there. It fails here: createFrontendPlugin re-wraps route refs, so the object the plugin holds is not the one routes.ts exported — probed it and the wrapped one carries neither `id` nor `params`. Identity happens to survive for a plugin with a single route and does not for this one, which is a property of the wrapping rather than of the plugin being correct. Asserting presence still catches removal, which the mutation above confirms. Part of RHIDP-16455.
gustavolira
requested review from
a team,
debsmita1,
its-mitesh-kumar and
rm3l
as code owners
August 27, 2026 19:18
Changed Packages
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4484 +/- ##
=======================================
Coverage 61.62% 61.62%
=======================================
Files 2569 2569
Lines 102831 102831
Branches 28837 28837
=======================================
Hits 63367 63367
Misses 38909 38909
Partials 555 555
*This pull request uses carry forward flags. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
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
1 participant
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.
nfsExports.test.tsasserts only that the translations module exists and is exported as default. That module is a separateFrontendModulewithpluginId: 'app', so both assertions hold against a bulk-import plugin whose ownextensionsarray is empty — the page and the API disappear from the app with no error, no console warning, exit 0.Same shape as #4470 for adoption-insights, which merged earlier today.
What's added
Three tests reading what the app actually resolves off the extensions:
routePathandtitletaskssubRouteRefMutation-verified
Each mutation applied to
src/index.tsx, suite re-run, source restored:extensionsextensionspath: '/bulk-import'changedtitle: 'Bulk import'changedrouteRef: rootRouteRefremovedtasks: importHistoryRouteRefremovedSix for six, and the counts separate the tests: dropping the page fails two, dropping the api fails only the registration one, dropping the subroute fails only the routes one.
One finding worth carrying to the next plugin
The adoption-insights test asserts
toBe(rootRouteRef)on the route ref and passes. The same assertion fails here.createFrontendPluginre-wraps route refs — probed it, and the object the plugin holds carries neitheridnorparams, so it is not the oneroutes.tsexported:Identity happens to survive for a plugin with a single route and does not for one that also declares a subRouteRef. That is a property of the wrapping, not of the plugin being correct, so this test asserts presence instead — which the mutation table above confirms still catches removal.
Flagging it because RHIDP-16455 has ten more plugins to go and
toBewould look like the established pattern.Verification
From
workspaces/bulk-import:yarn tsc,yarn lintandyarn prettier --checkall clean; the suite runs 5 tests.@backstage/frontend-test-utilsis added at^0.6.1, the range four other workspaces use at this manifest version.Part of RHIDP-16455.