test(quantum): Vitest coverage for useQuantumCircuitAscii hook glue (#13832)#13852
Conversation
|
[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!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
👋 Hey @ANAMASGARD — thanks for opening this PR!
This is an automated message. |
There was a problem hiding this comment.
Pull request overview
Adds Vitest unit tests for two quantum hooks (useQuantumAuthStatus and useQuantumCircuitAscii) from the useCachedQuantum module, covering disabled-path behavior and isDemoData derivation logic.
Changes:
- New test file for
useQuantumAuthStatuscovering disabled path and threeisDemoDatacases. - New test file for
useQuantumCircuitAsciicovering disabled path and demo-mode data return.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| web/src/hooks/tests/useQuantumAuthStatus.test.ts | Adds 2 tests validating disabled return + isDemoData = isDemoFallback && !isLoading logic |
| web/src/hooks/tests/useQuantumCircuitAscii.test.ts | Adds 2 tests validating disabled return + demo-mode DEMO_QUANTUM_CIRCUIT data |
…uilds Revert duplicate declarations and invalid writeJSON err checks introduced on main by PR kubestellar#13816 so CI can build the full repo for kubestellar#13852. Signed-off-by: Gaurav Chaudhary <chaudharygaurav2004@gmail.com>
571ff0a to
d8917b9
Compare
There was a problem hiding this comment.
Thanks for the test coverage contribution! However this PR has significant scope creep beyond the stated test additions:
Files that shouldn't be in a test PR:
App.tsx— removesLiveLocationProvider(functional change)NamespaceOverview.tsx— removeshasShownRestoreErrorRef(functional change)Layout.tsx— adds new importClusterFilterPanel.tsx— adds new importuseMissions.provider.tsx,useStellar.tsx,kagentBackend.ts,localAgentChat.ts,services/stellar.ts— unclear changes
What's good: The two new test files (useQuantumAuthStatus.test.ts and useQuantumCircuitAscii.test.ts) look well-structured.
Please:
- Remove all non-test changes from this PR (revert to only the two new test files)
- If the other changes fix real issues, open a separate PR for each
The non-test changes conflict with main and risk introducing regressions. Keep PRs focused on one concern.
|
@kubestellar-hive[bot]: changing LGTM is restricted to collaborators DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
d6147ee to
7d5eb26
Compare
There was a problem hiding this comment.
Test coverage here is a bit too mock-driven to catch real regressions.
- Both specs stub
useCachecompletely, but never assert the contract passed into it (key,enabled,initialData,demoData,fetcher,refreshInterval,autoRefresh). That means a wiring bug insideuseQuantumAuthStatus/useQuantumCircuitAsciicould still pass as long as the stubbed return shape looks right. useQuantumAuthStatusshould explicitly assert the unauthenticated branch returnsDEFAULT_AUTH_STATUS, preserves the mockedrefetch, and disables the cache (enabled: false). Right now it only checks derived fields from a fabricated cache result.useQuantumCircuitAsciishould exact-matchDEMO_QUANTUM_CIRCUITand add aisLoading=true && isDemoFallback=truecase to pin the "no demo data while loading" rule. A regex on the ASCII art is too loose to prove the hook returned the intended demo object.vi.resetAllMocks()is brittle here because it wipes the module mock implementation for the polling context; that makesautoRefreshbehavior depend on an implicitundefinedcoercion instead of the intended mocked value. Prefervi.clearAllMocks()or re-stub the polling mock in each test so the suite stays deterministic.- Neither file covers
forceDemo, so a regression in that guard would be missed.
Net: good start, but I would tighten these into contract/edge-case tests so they fail on real hook regressions, not just snapshot the mocked outputs.
|
Hi @ANAMASGARD! Thanks for contributing these tests for The build failures here are not caused by your test code — your branch is stale against
Fix: rebase your branch onto the current git fetch origin main
git rebase origin/main
# resolve any conflicts, then:
git push --force-with-leaseOnce you rebase, CI should pick up the current |
There was a problem hiding this comment.
Hi @ANAMASGARD — several blocking CI checks are failing on this PR:
- build (both amd64 and arm64)
- fullstack-smoke
- pr-check
- All Cards TTFI (Hard Gate)
- App Visual Regression
Please investigate the build failure (likely a TypeScript compilation error), fix the issues, and push an updated commit. You may also need to rebase on the latest main since several PRs have merged recently. Thanks!
|
@kubestellar-hive[bot]: changing LGTM is restricted to collaborators DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Thanks for the update. Current failing checks are fullstack-smoke and pr-check.\n\nPlease run a rebase onto latest |
There was a problem hiding this comment.
Hey @ANAMASGARD — this PR has merge conflicts with main and CI is failing. Could you:
- Rebase on the latest main:
git fetch origin main && git rebase origin/main - Resolve conflicts and force-push
- Check the CI failures after rebase
Happy to re-review once it's green. Thanks!
There was a problem hiding this comment.
Hey @ANAMASGARD 👋 — this PR currently has a merge conflict and multiple CI failures (build, fullstack-smoke, pr-check, TTFI, visual regression all fail).
Could you please:
- Rebase on the latest
main:git fetch origin main && git rebase origin/main - Fix the build errors — likely due to upstream changes since this branch was created
- Force-push the updated branch
Happy to re-review once CI is green. Thanks!
|
👋 @ANAMASGARD — This PR has merge conflicts and a CI build failure. Could you rebase onto |
|
@kubestellar-hive[bot]: changing LGTM is restricted to collaborators DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@kubestellar-hive[bot]: changing LGTM is restricted to collaborators DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
There was a problem hiding this comment.
Hi @ANAMASGARD — this PR has a merge conflict and CI is failing (build/fullstack-smoke). Could you rebase on main and fix the build failures? Thanks for the contribution!
|
Following up on the earlier rebase request — the branch is still conflicting with Please rebase onto current git fetch origin main
git rebase origin/main
git push --force-with-leaseThe test logic itself looks reasonable once the branch is current. Happy to review again after the rebase! |
|
Hi @ANAMASGARD — the test additions look good structurally, but CI is failing across multiple jobs. Root cause: Branch is 63 commits behind Fix: Rebase onto current git fetch origin
git rebase origin/main
# Resolve any conflicts
git push --force-with-leaseThe test files themselves ( Failing checks:
All are build-stage failures, not test failures. Rebase will resolve. |
|
@kubestellar-hive[bot]: changing LGTM is restricted to collaborators DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
…ubestellar#13832) Add Vitest glue tests for useQuantumCircuitAscii; rebase drops duplicate useQuantumAuthStatus coverage already merged in kubestellar#13911. Signed-off-by: Gaurav Chaudhary <chaudharygaurav2004@gmail.com>
…act (kubestellar#13832) - Add UseCacheResult fields (retryFetch, clearAndRefetch) to mocks - Strengthen disabled-path test with noisy cache + useCache wiring asserts - Exact-match DEMO_QUANTUM_CIRCUIT; pin isDemoData when loading+demo - Cover forceDemo disabling cache; use clearAllMocks in beforeEach Signed-off-by: Gaurav Chaudhary <chaudharygaurav2004@gmail.com>
7d5eb26 to
c4274d7
Compare
|
I have done all that rebase and things you asked for . |
cb0b9f4
into
kubestellar:main
Part of #13832
Adds Vitest tests for the
useQuantumCircuitAsciiglue layer:isAuthenticated: falsereturns null data with flags normalized viagetDisabledResult,refetchforwarded, anduseCachewired withenabled: falseand expected key/demo/fetcherisDemoFallback+ not loading returnsDEMO_QUANTUM_CIRCUITwithisDemoData: true(exact object match)isDemoFallback: truewhileisLoading: truekeepsisDemoDatafalseforceDemo: cacheenabled: falsewhen authenticated withforceDemo: trueNote:
useQuantumAuthStatustests were merged in #13911 — this PR only addsuseQuantumCircuitAscii.test.ts.Out of scope: other quantum UI tests per #13832.
git commit -s)