Skip to content

fix(#475): seed E2E state through the app instead of poking the QueryClient - #490

Closed
jarik2014 wants to merge 2 commits into
SmartDropLabs:mainfrom
jarik2014:fix/475-e2e-seed-hook
Closed

jarik2014 wants to merge 2 commits into
SmartDropLabs:mainfrom
jarik2014:fix/475-e2e-seed-hook

Conversation

@jarik2014

Copy link
Copy Markdown

Closes #475

What changes

e2e/farm.spec.ts seeds React Query state by reaching for window.__queryClient and calling setQueryData(['pools'], …) itself. The issue is right that this breaks the day the global goes away — and it also couples the spec to the literal spelling of every query key, so a rename in useSorobanQuery fails a test in a file that has nothing to do with the change.

It now seeds through a small surface the app owns:

window.__e2e.seedPools([pool]);
window.__e2e.seedPositions(publicKey, [{ pool, position }]);
window.__e2e.clearPositions(publicKey);
window.__e2e.reset();

src/lib/e2eSeed.ts builds those keys from QUERY_KEYS, which lives next to the hooks that read them, into one place. src/context/index.tsx installs it under the same development / NEXT_PUBLIC_E2E condition the raw client was exposed under, and no longer publishes window.__queryClient at all.

e2e/lock-unlock-lifecycle.spec.ts used the same global for the same purpose, so it is migrated as well — leaving it coupled to an internal the app no longer offers would just be a second version of this issue.

Tests

src/lib/e2eSeed.test.ts, 9 new tests:

  • pools land under the key useSorobanQuery reads;
  • positions land under the key the app invalidates, ['userPosition', 'all', publicKey];
  • positions are keyed per account, so a seed for one wallet cannot leak into another;
  • clearing positions leaves the pool list alone; reset clears both;
  • the key spellings the specs depend on are pinned, so a rename fails here — next to the reason — rather than inside an unrelated spec;
  • no file under e2e/ or tests/ mentions __queryClient, and src/context/index.tsx no longer hands it out. That is the issue's requirement asserted as a test instead of trusted to review.

Verification

$ pnpm exec tsc --noEmit                        # clean
$ pnpm exec vitest run src/lib/e2eSeed.test.ts  # 9 passed
$ pnpm test                                     # 357 passed, 23 failed (see below)

The 23 failures are pre-existing and unrelated: with these changes stashed, the same suite reports 348 passed and the same 23 failures in the same files (soroban-parsers, feeBumpGuard, useLeaderboard, Navbar, alerts, StellarWalletContext, soroban.service, useSorobanEvents). The delta is exactly the 9 tests added here.

eslint on the changed files reports 0 errors and two pre-existing POOLS_XDR is assigned a value but never used warnings — both constants are unused on main too, and I left them alone rather than bundling an unrelated cleanup into this PR.

One thing I found but did not touch

pnpm install --frozen-lockfile fails on main: package.json lists next-intl and pnpm-lock.yaml does not, so a locked install cannot resolve. I installed without --frozen-lockfile to run the checks and left the lockfile exactly as it was. Worth its own fix, since it means a clean checkout cannot do a reproducible install.

I have not run the Playwright specs against a real browser yet — doing that next and will report the result here.

…ing the QueryClient

`e2e/farm.spec.ts` seeded React Query data by reaching for `window.__queryClient`
and calling `setQueryData(['pools'], …)` itself. That couples every spec to two
things it should not know about: the QueryClient instance, and the literal
spelling of each query key. Rename a key and an unrelated spec fails; drop the
global and every spec breaks at once — which is exactly what the issue reports.

`src/lib/e2eSeed.ts` owns that knowledge instead and exposes a small surface:

  window.__e2e.seedPools([pool])
  window.__e2e.seedPositions(publicKey, [{ pool, position }])
  window.__e2e.clearPositions(publicKey)
  window.__e2e.reset()

The keys come from `QUERY_KEYS` in `useSorobanQuery`, which is where the hooks
that read them are defined, so a rename moves both sides together.

`src/context/index.tsx` installs it under the same development / NEXT_PUBLIC_E2E
condition the raw client used to be exposed under, and no longer publishes
`window.__queryClient` at all. `e2e/lock-unlock-lifecycle.spec.ts` used the same
global for the same reason, so it is migrated too rather than leaving one spec
coupled to an internal the app no longer offers.

Tests — `src/lib/e2eSeed.test.ts`, 9 new:

- pools land under the key `useSorobanQuery` reads;
- positions land under the key the app invalidates, `['userPosition', 'all', pk]`;
- positions are per account, so one seed cannot leak into another;
- clearing positions leaves the pool list alone, and reset clears both;
- the key spellings the specs rely on are pinned, so a rename fails next to the
  reason instead of inside an unrelated spec;
- no file under `e2e/` or `tests/` mentions `__queryClient` any more, and
  `src/context/index.tsx` no longer hands it out — the same invariant, asserted
  rather than trusted.

Verification:

  pnpm exec tsc --noEmit            # clean
  pnpm exec eslint <changed files>  # 0 errors (2 pre-existing unused-const warnings)
  pnpm exec vitest run src/lib/e2eSeed.test.ts   # 9 passed
  pnpm test                         # 357 passed, 23 failed

The 23 failures are pre-existing: stashing these changes and running the same
suite gives 348 passed and the same 23 failures in the same files, so this commit
adds 9 passing tests and changes nothing else.

Found on the way, unrelated to this issue: `pnpm install --frozen-lockfile` fails
on `main` because `package.json` lists `next-intl` and `pnpm-lock.yaml` does not,
so a locked install cannot resolve. Worth fixing separately — the lockfile here is
untouched by this commit.
@netlify

netlify Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for spiffy-melomakarona-eb1e8a ready!

Name Link
🔨 Latest commit b228bf1
🔍 Latest deploy log https://app.netlify.com/projects/spiffy-melomakarona-eb1e8a/deploys/6ab549de1214c7000884891f
😎 Deploy Preview https://deploy-preview-490--spiffy-melomakarona-eb1e8a.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@netlify

netlify Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

❌ Deploy Preview for smart-drop failed.

Name Link
🔨 Latest commit b228bf1
🔍 Latest deploy log https://app.netlify.com/projects/smart-drop/deploys/6ab549de08c9ae000844b467

@jarik2014

Copy link
Copy Markdown
Author

Ran the spec in a real browser. With #492's two fixes applied and this change in place:

$ pnpm exec playwright test --config e2e/playwright.config.ts e2e/farm.spec.ts
  ✓  1 · connect wallet — header updates to show truncated address (8.2s)
  ✓  2 · deposit — modal accepts 10 XLM and submits (7.8s)
  ✓  3 · countdown visible — Unlock button is disabled before lock period (2.2s)
  ✓  4 · unlock available — fast-forward 8 days enables Unlock (4.3s)
  ✓  5 · unlock — fill modal, sign, submit; stake drops to 0 (3.4s)
  5 passed (45.9s)

That matters for this PR specifically: tests 3–5 are the seeded ones — they read the pool list and the locked position that seedPools/seedPositions write through the new surface, so the change is exercised end to end and not only by the unit tests.

To be precise about what the run does and does not show: on main these five failed with a build-error overlay on /farm (the duplicate stepRef, unrelated to this change — stashing this change reproduces those same five failures), so the pass is not this PR alone. What it shows is that with the app able to run, the new seeding path works and the specs are not left behind.

@jarik2014

Copy link
Copy Markdown
Author

Added one more commit: a lockfile sync that is not about this PR's own subject, but is the reason every Netlify check on this PR was red.

package.json has carried next-intl: ^4.14.6 while pnpm-lock.yaml never got it. Netlify installs with --frozen-lockfile, so the install fails before the build is reached:

ERR_PNPM_OUTDATED_LOCKFILE  Cannot install with "frozen-lockfile" because
pnpm-lock.yaml is not up to date with <ROOT>/package.json
    Failure reason: specifiers in the lockfile (...) don't match specs in package.json (...)
    the only difference is next-intl

That is why the six checks on this PR ("Header rules", "Pages changed", "Redirect rules" — the two deploy previews each report three) were failing: they report the preview failing to build, and the preview cannot build while the install is refused. It is repo-wide, not specific to this branch.

The commit adds next-intl and its dependency tree to the lockfile and nothing else — pnpm install --no-frozen-lockfile reports no specifier changes for existing packages, the 439 added lines are all entries the package needs (icu-minify, intl-messageformat, negotiator, po-parser, the SWC extractor plugin). Verified after the change:

$ pnpm install --frozen-lockfile
Already up to date          # the exact command that used to abort

$ pnpm build
…compiled successfully, full route table printed, middleware 34 kB

The dependency is real, not stale: src/i18n.ts and src/request.ts both import { getRequestConfig } from "next-intl/server".

This commit is also on the other three open PRs from the same base (#490, #491, #493) so their previews can go green independently of this one merging.

@jarik2014

Copy link
Copy Markdown
Author

Update on the checks, since they now tell a clearer story: #492 is fully green (both deploy previews, header/redirect rules), and this one is still red — but not for the reason it was.

This branch carries the lockfile sync, so the install no longer aborts. What is left is the defects that live on main itself — the missing useEffect import in Navbar.tsx, the duplicated stepRef, the SkeletonBar prop type, the locale guard cast. They are not introduced by this PR; they are why every deploy preview in this repository fails, including this one's. #492 removes all four, which is why #492 builds.

I am deliberately not copying those fixes here: they would duplicate #492, and when it merges this branch picks them up on rebase. If you would rather each PR stand alone and be green today, say so and I will cherry-pick them in.

@jarik2014

Copy link
Copy Markdown
Author

The issue this targets (#475) has been completed by @SweetBoy-eth and the wave bot has already recorded the credit against that PR, so this branch can no longer earn anything for anyone. Closing it to keep the queue clean rather than leaving a conflicting PR open.

@jarik2014 jarik2014 closed this Sep 25, 2026
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.

[bug] E2E farm.spec.ts uses window.__queryClient for test seeding

1 participant