vitest: Clean up the test output (HMS-10742)

[regexowl]

This updates tests to clean up the test output and fixes the ImagesTable test in npm run test:cockpit.

Rebased on #4426

JIRA: HMS-10742

[regexowl]

This is mostly an idea. Usually some "missing act()" warnings sneak in without noticing and the output becomes a mess. If we fail on those, the problems could be addressed directly in the PR.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.34%. Comparing base (46a51fc) to head (bd224c7).

@@            Coverage Diff             @@
##             main    #4469      +/-   ##
==========================================
+ Coverage   75.31%   75.34%   +0.03%     
==========================================
  Files         229      229              
  Lines        7449     7446       -3     
  Branches     2768     2768              
==========================================
  Hits         5610     5610              
+ Misses       1581     1578       -3     
  Partials      258      258              

Files with missing lines	Coverage Δ
...ps/Registration/components/ManualActivationKey.tsx	8.69% <100.00%> (+1.00%)	⬆️

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 46a51fc...bd224c7. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[regexowl]

/jira-epic HMS-10502

[ochosi]

/jira-epic HMS-10502

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• In the Repositories component, the effect uses reposInTemplate but omits it from the dependency array (with an eslint disable); consider restructuring this logic so you can include all referenced values in the dependencies and avoid potential stale state issues instead of suppressing the lint rule.
• The change to generateFillerRepos now produces a fresh UUID with uuidv4() for each repo, which can introduce non-determinism into tests; consider using a deterministic ID based on the index (e.g. filler-repo-${i}) so test output and snapshots remain stable while still avoiding duplicate keys.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In the Repositories component, the effect uses `reposInTemplate` but omits it from the dependency array (with an eslint disable); consider restructuring this logic so you can include all referenced values in the dependencies and avoid potential stale state issues instead of suppressing the lint rule.
- The change to `generateFillerRepos` now produces a fresh UUID with `uuidv4()` for each repo, which can introduce non-determinism into tests; consider using a deterministic ID based on the index (e.g. `filler-repo-${i}`) so test output and snapshots remain stable while still avoiding duplicate keys.

## Individual Comments

### Comment 1
<location path="src/Components/CreateImageWizard/steps/Registration/components/ManualActivationKey.tsx" line_range="45-51" />
<code_context>
-      <FormGroupLabelHelp
-        ref={ref}
-        aria-label='About Activation Keys & Organization ID'
+      <Button
+        icon={<HelpIcon />}
+        variant='plain'
+        size='sm'
+        aria-label='Credentials path info'
+        hasNoPadding
       />
     </Popover>
</code_context>
<issue_to_address>
**suggestion:** Align the button aria-label with the popover header for clearer accessibility semantics.

The button uses `aria-label='Credentials path info'` while the popover header is `About Activation Keys & Organization ID`, which can sound unrelated for screen-reader users. Please align the `aria-label` text with the header (or make it very similar) so it’s clear this control opens that specific popover.

```suggestion
      <Button
        icon={<HelpIcon />}
        variant='plain'
        size='sm'
        aria-label='About Activation Keys & Organization ID'
        hasNoPadding
      />
```
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[regexowl]

• The change to generateFillerRepos now produces a fresh UUID with uuidv4() for each repo, which can introduce non-determinism into tests; consider using a deterministic ID based on the index (e.g. filler-repo-${i}) so test output and snapshots remain stable while still avoiding duplicate keys.

Also addressed.

[mgold1234]

Would it be possible to restructure this to avoid the eslint-disable? For example, using a ref to capture reposInTemplate so the effect can safely include all its dependencies.

[regexowl]

Ok, I think this is finally ready for review. Found some more loops while trying to clean up the test output.

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• The useEffect that removes unavailable repositories depends only on hasRemovedUnavailable, previousLoading, and previousSuccess but uses unavailableRepoIds and removeSelected, so it should include those in the dependency array to avoid stale values and ensure React hook rules are followed.
• Because hasRemovedUnavailable is only initialized once and never reset when initialSelectedState or previousReposData change, the cleanup effect for unavailable repositories will not rerun on subsequent changes; consider deriving this flag from props/data or resetting it when the selection or previous repo data change so new unavailable entries are also removed.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The `useEffect` that removes unavailable repositories depends only on `hasRemovedUnavailable`, `previousLoading`, and `previousSuccess` but uses `unavailableRepoIds` and `removeSelected`, so it should include those in the dependency array to avoid stale values and ensure React hook rules are followed.
- Because `hasRemovedUnavailable` is only initialized once and never reset when `initialSelectedState` or `previousReposData` change, the cleanup effect for unavailable repositories will not rerun on subsequent changes; consider deriving this flag from props/data or resetting it when the selection or previous repo data change so new unavailable entries are also removed.

## Individual Comments

### Comment 1
<location path="src/Components/CreateImageWizard/steps/Repositories/components/Repositories.tsx" line_range="267-276" />
<code_context>
-  const orgIdRef = useRef(null);
-  const activationKeyRef = useRef(null);

   useEffect(() => {
     dispatch(changeServerUrl('subscription.rhsm.redhat.com'));
</code_context>
<issue_to_address>
**issue (bug_risk):** The `useEffect` removing unavailable repos is missing `unavailableRepoIds` in its dependency array.

Because the effect reads `unavailableRepoIds` but it’s not in the dependency array (only `hasRemovedUnavailable`, `previousLoading`, `previousSuccess` are), the effect may not rerun when `unavailableRepoIds` changes (e.g., via `previousReposData` or `initialSelectedState`), leaving some unavailable repos in the selection.

Add `unavailableRepoIds` (and possibly `removeSelected` if required by ESLint) to the dependency array so the effect runs when it first becomes non-empty:

```ts
useEffect(() => {
  if (hasRemovedUnavailable || unavailableRepoIds.length === 0) {
    return;
  }

  removeSelected(
    unavailableRepoIds.map((uuid) => ({ uuid })) as ApiRepositoryResponseRead[],
  );
  setHasRemovedUnavailable(true);
}, [
  hasRemovedUnavailable,
  unavailableRepoIds,
  previousLoading,
  previousSuccess,
]);
```

You can still keep `hasRemovedUnavailable` to ensure this only runs once per session; the key is that the effect must react to the first non-empty `unavailableRepoIds` value.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[kingsleyzissou]

Let's go 🚀

[regexowl]

Ugh, even more loops. Looks like a relevant failure, so back to the draft it goes ._.

[regexowl]

Hmm, I'll pivot a bit. Will update the PR to not touch the loops and to not fail on warn/error. Just cleaning up the warnings and errors for now. And I'll open a fresh one that will focus on the loops only where I could push without pinging everyone all the time...

[regexowl]

And this will be the follow up #4489

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• In ManualActivationKey, consider wiring the Button inside ManualRegistrationPopover through a triggerRef passed to Popover (similar to the previous FormGroupLabelHelp approach) so the popover anchor/positioning and ARIA wiring remain explicit and robust.
• In src/test/mocks/cockpit/fsinfo.ts, the hardcoded path comparison for the compose blueprint could reuse the bpDir constant (e.g., via path.join(bpDir, 'b3ff...')) to avoid duplicating the directory string and to keep future path changes centralized.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In `ManualActivationKey`, consider wiring the `Button` inside `ManualRegistrationPopover` through a `triggerRef` passed to `Popover` (similar to the previous `FormGroupLabelHelp` approach) so the popover anchor/positioning and ARIA wiring remain explicit and robust.
- In `src/test/mocks/cockpit/fsinfo.ts`, the hardcoded path comparison for the compose blueprint could reuse the `bpDir` constant (e.g., via `path.join(bpDir, 'b3ff...')`) to avoid duplicating the directory string and to keep future path changes centralized.

## Individual Comments

### Comment 1
<location path="src/Components/CreateImageWizard/steps/Registration/components/ManualActivationKey.tsx" line_range="45-50" />
<code_context>
     >
-      <FormGroupLabelHelp
-        ref={ref}
+      <Button
+        icon={<HelpIcon />}
+        variant='plain'
+        size='sm'
         aria-label='About Activation Keys & Organization ID'
+        hasNoPadding
       />
     </Popover>
</code_context>
<issue_to_address>
**issue (bug_risk):** The `hasNoPadding` prop on `Button` is likely unsupported and may cause type or runtime issues.

This prop exists on some toggle components but not on `Button`, so it will either cause a TypeScript error or be ignored. For a compact icon-only button, rely on `variant="plain"` and `size="sm"`, and if you still need spacing tweaks, use a utility class instead of `hasNoPadding`.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

issue (bug_risk): The hasNoPadding prop on Button is likely unsupported and may cause type or runtime issues.

This prop exists on some toggle components but not on Button, so it will either cause a TypeScript error or be ignored. For a compact icon-only button, rely on variant="plain" and size="sm", and if you still need spacing tweaks, use a utility class instead of hasNoPadding.

[regexowl]

/retest