🐛 fix(tests): tighten ComplianceScoreBreakdownModal test scoping (#7908)#7909
🐛 fix(tests): tighten ComplianceScoreBreakdownModal test scoping (#7908)#7909clubanderson merged 1 commit intomainfrom
Conversation
Two Copilot review comments on merged PR #7905: 1. **Line 400** — `getAllByText('Kubescape').length >= 2` / `getAllByText('Kyverno').length >= 2` are too loose. Any extra rendering of those names anywhere in the modal would satisfy the assertion, and the check doesn't actually verify the per-tool bars exist. Replaced with a scoped lookup via the "By tool" heading: grab the section container and use `within(section).getByText(...)` so the assertion only accepts matches inside the per-tool list. 2. **Line 512** — `getByText('100')` is a broad assertion; any element rendering "100" anywhere (score %, count, label) would pass. Replaced with a value-label pairing check: locate the "Total Checks" label, walk up to its StatBox parent, and assert "100" appears inside that StatBox. Added `within` to the @testing-library/react import. All 12 tests in the file continue to pass locally; `npm run build` green. Fixes #7908 Signed-off-by: Andy Anderson <andy@clubanderson.com>
kubestellar-prow
Bot
added
the
dco-signoff: yes
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
✅ Deploy Preview for kubestellarconsole ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
👋 Hey @clubanderson — thanks for opening this PR!
This is an automated message. |
kubestellar-prow
Bot
added
the
size/S
There was a problem hiding this comment.
Pull request overview
Tightens ComplianceScoreBreakdownModal unit test assertions in the web frontend to avoid false positives as the modal UI evolves, addressing follow-up review feedback from the previously merged test fix.
Changes:
- Scope “Kubescape/Kyverno” assertions to the “By tool” section using
within(...)instead of globalgetAllByText(...)length checks. - Assert the “Total Checks” value (“100”) within the corresponding StatBox container to ensure correct label/value pairing.
- Add
withinto the@testing-library/reactimport to support the narrowed assertions.
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
|
Post-merge build verification passed ✅ Both Go and frontend builds compiled successfully against merge commit |
Summary
Addresses the two Copilot review comments left on merged PR #7905 for
web/src/components/cards/ComplianceCards.test.tsx. Both comments flagged assertions as too loose and prone to false positives as the UI evolves.1. Scope "By tool" assertions to the per-tool section
Before:
```ts
expect(screen.getAllByText('Kubescape').length).toBeGreaterThanOrEqual(2)
expect(screen.getAllByText('Kyverno').length).toBeGreaterThanOrEqual(2)
```
After:
```ts
const byToolHeading = screen.getByText('By tool')
const byToolSection = byToolHeading.parentElement!
expect(within(byToolSection).getByText('Kubescape')).toBeInTheDocument()
expect(within(byToolSection).getByText('Kyverno')).toBeInTheDocument()
```
2. Assert Total Checks value-label pairing
Before:
```ts
expect(screen.getByText('Total Checks')).toBeInTheDocument()
expect(screen.getByText('100')).toBeInTheDocument()
```
After:
```ts
const totalChecksLabel = screen.getByText('Total Checks')
const totalChecksStatBox = totalChecksLabel.parentElement!
expect(within(totalChecksStatBox).getByText('100')).toBeInTheDocument()
```
Added
withinto the@testing-library/reactimport.Fixes #7908
Test plan
npx vitest run src/components/cards/ComplianceCards.test.tsx— 12/12 passnpm run buildpasses