🌱 test: add tests for useQASMFiles hook#13484
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 @Shruti2110-coder — thanks for opening this PR!
This is an automated message. |
|
👋 Welcome to the KubeStellar community! 💖 Thanks and congrats 🎉 for opening your first PR here! We're excited to have you contributing. Before merge, please ensure:
📬 If you're using KubeStellar in your organization, please add your name to our Adopters list. 🙏 It really helps the project gain momentum and credibility — a small contribution back with a big impact. Resources:
A maintainer will review your PR soon. Hope you have a great time here! 🌟 ~~~~~~~~~~ 🌟 📬 If you like KubeStellar, please ⭐ star ⭐ our repo to support it! 🙏 It really helps the project gain momentum and credibility — a small contribution back with a big impact. |
1579808 to
320a464
Compare
Signed-off-by: Shruti2110-coder <jainshruti1021@gmail.com> Signed-off-by: Shruti2110-coder <your-github-email@example.com>
There was a problem hiding this comment.
Security review: PASS ✅
Test-only changes:
useQASMFiles.test.ts: Refactored test suite with better structure (225 additions, 88 deletions). Usesvi.fn()mocks onglobalThis.fetch— standard Vitest pattern. No production code changes.mockServiceWorker.js: MSW version bump 2.13.6 → 2.14.6 (auto-generated file).package-lock.json: Dependency resolution changes only.
No secrets, no unsafe patterns, no production code modifications.
|
@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.
Thanks for contributing tests for useQASMFiles! 🙏
Unfortunately this PR has several CI failures that need to be fixed before it can be reviewed for merge:
1. ❌ Build Failure — broken package-lock.json
Your PR strips resolved and integrity fields from package-lock.json and updates some dependency versions. This causes npm ci to fail in the Docker build with:
Error: Cannot find native binding.
Cannot find module '@rolldown/binding-linux-x64-musl'
Fix: Revert your package-lock.json changes. Run npm install locally with the same Node version the project uses (check .nvmrc or the Dockerfile), then commit only the lockfile changes that are strictly necessary for your new test dependency (if any). If you didn't intentionally add a new dependency, simply restore the original package-lock.json from main.
2. ❌ DCO Sign-off Missing
All commits must include a Signed-off-by trailer per the DCO. Use:
git commit --amend -s
git push --force-with-lease3. ❌ PR Body Not Filled Out
The PR description uses the template but doesn't link to an issue. Please update the body to start with Fixes #ISSUE_NUMBER (the issue this test addresses), or explain the motivation if there's no issue.
4. ⚠️ Test Quality Notes
The test file itself looks well-structured — good use of helpers, clear grouping by behavior, and meaningful assertions. One concern: the existing test on main already covers the demo-mode case returning [{ name: 'bell.qasm' }], but your version expects an empty array. Please verify which behavior is correct by checking the actual hook implementation.
Next steps:
- Revert
package-lock.jsonto matchmain - Add DCO sign-off (
git commit --amend -s) - Fill in the PR body with the linked issue
- Force-push — CI will re-run automatically
Happy to help if you have questions!
|
@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. |
Signed-off-by: Shruti2110-coder <your-github-email@example.com>
320a464 to
8997783
Compare
|
The CI failures in Card Loading Standard and UI/UX Standards appear to be |
Signed-off-by: Shruti2110-coder <your-github-email@example.com>
There was a problem hiding this comment.
Thanks for adding coverage for useQASMFiles — the assertions are generally much more meaningful now. I do see two things worth tightening before approval:
-
The hook has a second input,
forceDemo, and that behavior is still untested. Right now the suite coversisQuantumForcedToDemo(), but not the explicit override path (useQASMFiles(..., true/false)) or its precedence over workload demo detection. Since that is real hook logic, I’d add targeted tests for it. -
This PR also includes a very large
web/package-lock.jsonrewrite plus amockServiceWorker.jsversion bump, which looks unrelated to adding hook tests and is also showing up in failing CI. If those changes were incidental, please drop them so the PR stays focused and easier to review.
Once those are addressed, this looks close.
Signed-off-by: Shruti2110-coder <your-github-email@example.com>
There was a problem hiding this comment.
Security Review: PASS ✅
Test-only PR (useQASMFiles hook tests). No production code changes, no new endpoints, no secrets, no user input handling. Safe to merge.
- No security-sensitive changes
- Mock patterns are standard Vitest patterns
- No globalThis.fetch leaking to production
|
@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.
LGTM — good test coverage and well-structured test suite. One minor formatting nit (non-blocking):
The 'returns demo files in demo mode' test has inconsistent indentation (body at column 2 instead of column 4). Doesn't affect functionality but worth fixing for consistency with the rest of the file.
CI is green. Approving.
|
@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. |
0a3d598
into
kubestellar:main
✅ Post-Merge Verification: passedCommit: |
What this PR does
Adds a comprehensive test suite for the
useQASMFileshook.Tests added
Checklist