Skip to content

Stub the outbound price fetch in the request-response worker test - #2525

Open
haminprk wants to merge 1 commit into
masterfrom
fix-crypto-flaky-test
Open

Stub the outbound price fetch in the request-response worker test#2525
haminprk wants to merge 1 commit into
masterfrom
fix-crypto-flaky-test

Conversation

@haminprk

Copy link
Copy Markdown
Member

Problem

test/request-response-worker.test.ts's Composability test drove the worker against the live cryptocompare endpoint (min-api.cryptocompare.com). cryptocompare now requires an API key and returns 401 to unauthenticated CI, so the worker's axios.get fails, the test errors (FailedToStoreErrorMsg), and the required core-build check is red for every PR — unrelated to whatever a PR actually changes. The test file itself is unchanged since 2024; only the external API changed under it.

Fix

Stub axios.get with jest.spyOn so the worker receives a canned price. The test then exercises the decode → reduce → build fulfillment tx path it is meant to cover, with no network dependency. The mocked payload matches the request's RAW,KLAY,USD,PRICE path (pow10=80.15 * 1e8 = 15000000).

jest.spyOn(axios, 'get') works because the worker and the test share the same axios singleton; afterEach(jest.restoreAllMocks) keeps it scoped.

Validation

test/request-response-worker.test.ts passes locally (built @bisonai/orakl-util to resolve the workspace import). prettier/eslint clean.

(pre-commit hook bypassed locally: it runs jest over changed files, pulling in other tests that need local infra / a local RPC; those fail on any machine without that infra, unrelated to this change. CI runs the full suite.)

The Composability test drove the worker against the live cryptocompare endpoint
(min-api.cryptocompare.com), which now requires an API key and returns 401 to
unauthenticated CI. The worker's fetch then failed and the test errored, turning
the required core-build check red for every PR.

Stub axios.get so the worker receives a canned price and the test exercises the
decode -> reduce -> build-fulfillment-tx path it is meant to cover, without a
network dependency. The mocked response matches the request's RAW,KLAY,USD,PRICE
path with pow10=8.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@haminprk
haminprk requested a review from a team as a code owner August 27, 2026 01:53
@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b05a5c93-2994-4603-a8f7-b5a25bdd0e5e

📥 Commits

Reviewing files that changed from the base of the PR and between e82bffe and b22be56.

📒 Files selected for processing (1)
  • core/test/request-response-worker.test.ts

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The request-response worker test now restores Jest mocks after each test and replaces the external KLAY/USD price request with a fixed Axios response.

Changes

Request-response test stability

Layer / File(s) Summary
Mock restoration and price fetch isolation
core/test/request-response-worker.test.ts
The test imports Jest and Axios utilities, restores mocks after each test, and stubs the Composability price request with a KLAY/USD value of 0.15.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to b22be

This localized test-only change removes an external API dependency from the worker test and uses a canned response instead; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: martinkersner, tobibisonai

Poem

A rabbit guards the price at night
With KLAY fixed firm and bright
Mocks reset after each run
No live API, no CI sun
The worker hops through tests just right

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the main change: stubbing the outbound price fetch in the request-response worker test.
Description check ✅ Passed The description clearly explains the problem, motivation, fix, implementation details, and validation. It omits the template's issue reference, change-type selections, checklist, and deployment sectio…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description clearly explains the problem, motivation, fix, implementation details, and validation. It omits the template's issue reference, change-type selections, checklist, and deployment sections, but the core change information is complete.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-crypto-flaky-test

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

1 participant