🐛 [scanner] fix: handle null images in buildpacks coverage test#19336
Conversation
Signed-off-by: Hive Scanner <hive-scanner@kubestellar.local>
✅ Deploy Preview for kubestellarconsole ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
[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 |
|
👋 Hey @clubanderson — thanks for opening this PR!
This is an automated message. |
|
🐝 Hi @clubanderson! I'm Trusted users — org members and contributors with write access — can mention Automation may take a moment to start, and follow-up happens through workflow activity rather than chat replies. |
✅ Test Coverage CheckAll new source files in this PR have corresponding test files. Checked |
There was a problem hiding this comment.
Pull request overview
This PR fixes a failing buildpacks coverage test and resolves an infinite refetch loop in the useBuildpackImages MCP hook by treating an empty cached array as valid cached data (when a real cache timestamp exists).
Changes:
- Update cache validation in
useBuildpackImagesto usebuildpackCache.timestamp > 0(instead ofdata.length > 0) so cached empty arrays don’t trigger repeated refetches. - Fix the “null images in response” test mock to include
status: 200, matching the hook’s reliance onresponse.status.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
web/src/hooks/mcp/buildpacks.ts |
Fixes cache-validity logic to prevent infinite refetch when cached images are []. |
web/src/hooks/mcp/__tests__/buildpacks-coverage.test.ts |
Updates fetch mock to include an HTTP status code for the null-images test case. |
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
|
Post-merge build verification passed ✅ Both Go and frontend builds compiled successfully against merge commit |
✅ Post-Merge Verification: passedCommit: |
Fixes #19334
This PR fixes the failing test in the buildpacks-coverage test suite by making two changes:
Test fix: Added missing
status: 200property to the mock fetch response in the "null images in response" test. The hook checksresponse.status === 404, so the mock needs a status property.Hook fix: Fixed infinite loop caused by empty array not being considered valid cached data. Changed cache validation from
buildpackCache.data.length > 0tobuildpackCache.timestamp > 0. An empty array is valid cached data and should not trigger repeated refetches.Root cause: When the API returned
nullfor images (which correctly defaulted to[]viadata.images || []), the empty array was cached, but the cache validation logic considered it invalid becauselength === 0. This caused the useEffect to repeatedly callrefetch(), creating an infinite loop manifested as "Maximum update depth exceeded" errors.Testing: All 5 tests in buildpacks-coverage.test.ts now pass.