Skip to content

[No QA] Fix SubmitPlanWelcomeModal test by rendering inside NavigationContainer - #96067

Closed
MelvinBot wants to merge 4 commits into
mainfrom
claude-fixSubmitPlanWelcomeModalTestNavigation
Closed

[No QA] Fix SubmitPlanWelcomeModal test by rendering inside NavigationContainer#96067
MelvinBot wants to merge 4 commits into
mainfrom
claude-fixSubmitPlanWelcomeModalTestNavigation

Conversation

@MelvinBot

Copy link
Copy Markdown
Contributor

Explanation of Change

Main went red because tests/ui/components/SubmitPlanWelcomeModalTest.tsx (added in #95099) fails to render:

Couldn't find a navigation object. Is your component inside NavigationContainer?
  at useNavigation (@react-navigation/core/useNavigation.tsx)
  at useFocusEffect (@react-navigation/core/useFocusEffect.tsx)
  at usePressLoading (src/hooks/usePressLoading.ts:48)
  at FormAlertWithSubmitButton (src/components/FormAlertWithSubmitButton.tsx:126)

This is a logical merge conflict, not a bug in either PR on its own. SubmitPlanWelcomeModalFeatureTrainingContentFormAlertWithSubmitButtonusePressLoading, and usePressLoading now calls useFocusEffect, which requires a navigation context. #95099 was written/tested against a main where usePressLoading did not yet use useFocusEffect, so its test rendered the modal without a NavigationContainer. Once both changes landed on main, the test began throwing at render time.

Fix: render the modal inside a NavigationContainer + stack screen in the test, following the existing pattern in tests/ui/components/FeatureTrainingModalTest.tsx. This is a test-only change — there is no runtime behavior change, since in the app the modal is always mounted inside navigation.

Fixed Issues

$ #96038
PROPOSAL:

Tests

  1. Run npx jest tests/ui/components/SubmitPlanWelcomeModalTest.tsx
  2. Verify all 3 tests pass.

⚠️ Note: local test execution was not available in the environment where this PR was authored, so the test run above was not performed locally — CI (test / test) will validate it.

  • Verify that no errors appear in the JS console

Offline tests

N/A — test-only change, no runtime behavior change.

QA Steps

N/A — no user-facing change. [No QA]

  • Verify that no errors appear in the JS console

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
  • I followed proper code patterns
  • I verified there are no console errors

Screenshots/Videos

N/A — test-only change, no UI change.

Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>
@mkhutornyi

Copy link
Copy Markdown
Contributor

@MelvinBot fix failing tests

@mkhutornyi

Copy link
Copy Markdown
Contributor

@MelvinBot close this PR as already fixed on main

Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>
@MelvinBot

Copy link
Copy Markdown
Contributor Author

Fixed the failing Oxfmt check.

The formatter flagged tests/ui/components/SubmitPlanWelcomeModalTest.tsx: the renderModal JSX was broken across multiple lines, but at the repo's printWidth: 190 oxfmt collapses it onto a single line. Committed the reformatted version (formatting-only — no behavior change).

     function renderModal() {
-        return render(
-            <OnyxListItemProvider>
-                {withNavigation(<SubmitPlanWelcomeModal />)}
-            </OnyxListItemProvider>,
-        );
+        return render(<OnyxListItemProvider>{withNavigation(<SubmitPlanWelcomeModal />)}</OnyxListItemProvider>);
     }

The checklist check is the PR Author Checklist (unchecked boxes in the PR body) — that's resolved by ticking the items, not by a code change, so I left it alone. The test jobs were still pending, not failing; this change is purely cosmetic and doesn't affect them.

MelvinBot and others added 2 commits July 14, 2026 13:47
…rt coverage

Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>
…TestNavigation' into claude-fixSubmitPlanWelcomeModalTestNavigation

Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>
@MelvinBot

Copy link
Copy Markdown
Contributor Author

Fixed both real failures and pushed to the branch. Two separate tests were red, both fallout from #95099 landing on main:

1. SubmitPlanWelcomeModalTest (test job) — the "Get the free plan" assertion ran too early. FormAlertWithSubmitButton now routes onSubmit through usePressLoading, which defers the work by one macrotask (setTimeout(…, 0)) so the spinner can paint first. The test asserted synchronously right after fireEvent.press, so autoCreateSubmitWorkspace hadn't been called yet. Wrapped the assertion in waitFor.

2. ExportOnyxStateTest (test job) — the new nvp_submitMigrationModalShown NVP key (also added by #95099) wasn't sorted into any export bucket, so the "every ONYXKEYS value must be in one of the four buckets" test failed. It's a plain boolean "was the modal shown" flag — same class as the surrounding NVP_SEEN_*/NVP_DISMISSED_* keys — so I added it to safeOnyxKeys in ExportOnyxState/common.ts (alphabetical position, which the ordering test verifies).

Also committed the oxfmt formatting fix (the Oxfmt check failure).

Verified locally:

  • tests/ui/components/SubmitPlanWelcomeModalTest.tsx — 3/3 pass
  • tests/unit/ExportOnyxStateTest.ts — 25/25 pass
  • npm run fmt clean, typecheck clean

The remaining PR Author Checklist / checklist failure is just the unchecked checklist boxes — not something I can complete on your behalf.

@MelvinBot

Copy link
Copy Markdown
Contributor Author

Closing this PR — the fix already landed on main. 👍

@MelvinBot MelvinBot closed this Jul 14, 2026
@MelvinBot

Copy link
Copy Markdown
Contributor Author

No code fix is needed — the substantive checks are green. The one remaining red check can only be resolved by you.

Status

  • test / test (all 8 shards) — the jest run that renders SubmitPlanWelcomeModalTest.tsx now passes, so the NavigationContainer fix in this PR works.
  • typecheck, ESLint check, Oxfmt check, Storybook tests — all passing.
  • PR Author Checklist / checklist — the only failing check.

Why the checklist is red: it's not a code failure. The check reports 2 out of 38 boxes ticked, with the remaining author-verification items (e.g. "I followed proper code patterns" and the "Verify that no errors appear in the JS console" boxes) still unchecked. These are human-verification items, so I can't tick them for you.

To go green: check the remaining boxes in the PR description's PR Author Checklist once you've verified them, and the check will re-run and pass.

@MelvinBot

Copy link
Copy Markdown
Contributor Author

The only failing check on this PR is PR Author Checklist / checklist, which is a manual checklist item, not a code/test failure — there's no code fix to push for it.

Also worth noting: this PR is closed, so its checks won't be re-run regardless.

The actual CI checks (builds, validate, CLA, Snyk) all passed. If you reopen the PR and still see the checklist marked red, it just needs the author checklist boxes ticked manually.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants