{{ message }}
test: add unit tests for parseProtocol, speedometer, throttle, and composeSignals helpers#10577
Open
mahmoodhamdi wants to merge 1 commit intoaxios:v1.xfrom
Open
test: add unit tests for parseProtocol, speedometer, throttle, and composeSignals helpers#10577mahmoodhamdi wants to merge 1 commit intoaxios:v1.xfrom
mahmoodhamdi wants to merge 1 commit intoaxios:v1.xfrom
Conversation
…mposeSignals helpers Add missing unit test coverage for four helper utilities: - parseProtocol: protocol parsing from URLs including edge cases (data URIs, custom protocols, max length boundary) - speedometer: data transfer rate calculation with circular buffer - throttle: function throttling with flush support - composeSignals: AbortSignal composition with timeout handling These helpers are used by the core library and fetch adapter but had no dedicated unit tests.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 4 files
Confidence score: 4/5
- Test-related issues only; no direct product code impact, so overall merge risk appears low
tests/unit/helpers/composeSignals.test.jsleaves timers/subscriptions pending, which could slow or destabilize the test suite over time- Pay close attention to
tests/unit/helpers/composeSignals.test.jsandtests/unit/helpers/speedometer.test.js- improve timer cleanup and strengthen assertions to catch regressions.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/unit/helpers/composeSignals.test.js">
<violation number="1" location="tests/unit/helpers/composeSignals.test.js:18">
P2: Timeout-based composeSignals tests leave real timers pending because created signals are not unsubscribed, which can slow or destabilize test execution.</violation>
</file>
<file name="tests/unit/helpers/speedometer.test.js">
<violation number="1" location="tests/unit/helpers/speedometer.test.js:36">
P2: Speedometer rate tests use weak `> 0` assertions, so formula/unit regressions can pass undetected.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| }); | ||
|
|
||
| it('should return an AbortSignal when timeout is provided', () => { | ||
| const signal = composeSignals([], 5000); |
Contributor
There was a problem hiding this comment.
P2: Timeout-based composeSignals tests leave real timers pending because created signals are not unsubscribed, which can slow or destabilize test execution.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/unit/helpers/composeSignals.test.js, line 18:
<comment>Timeout-based composeSignals tests leave real timers pending because created signals are not unsubscribed, which can slow or destabilize test execution.</comment>
<file context>
@@ -0,0 +1,80 @@
+ });
+
+ it('should return an AbortSignal when timeout is provided', () => {
+ const signal = composeSignals([], 5000);
+ expect(signal).toBeDefined();
+ expect(signal.aborted).toBe(false);
</file context>
Contributor
There was a problem hiding this comment.
P2: Speedometer rate tests use weak > 0 assertions, so formula/unit regressions can pass undetected.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/unit/helpers/speedometer.test.js, line 36:
<comment>Speedometer rate tests use weak `> 0` assertions, so formula/unit regressions can pass undetected.</comment>
<file context>
@@ -0,0 +1,93 @@
+
+ expect(result).toBeDefined();
+ expect(typeof result).toBe('number');
+ expect(result).toBeGreaterThan(0);
+ });
+
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds missing unit test coverage for four helper utilities that had no dedicated tests:
parseProtocol— protocol parsing from URLs (http, https, ftp, data URIs, custom protocols, max length boundary)speedometer— data transfer rate calculation using circular buffer (default params, custom min, rate computation)throttle— function throttling decorator (immediate invocation, threshold enforcement, flush, multi-arg passthrough)composeSignals— AbortSignal composition with timeout (timeout abort, external signal forwarding, falsy signal filtering, unsubscribe)These helpers are used by the core library and fetch adapter but previously had no dedicated unit tests. This brings the tested helper count from 9/32 to 13/32.
Motivation
While working with the fetch adapter in a project, I noticed these core helpers lacked test coverage.
composeSignalsandthrottlein particular are critical for request timeout and progress event handling — having tests for them helps catch regressions during future changes.Test Details
parseProtocol+/-chars, data URIs, file protocol, no protocol, empty string, 25-char boundaryspeedometerthrottlecomposeSignalsTotal: 36 new tests, all passing.
Testing
npx vitest run tests/unit/helpers/ # 13 test files, 74 tests — all passingLint is clean (
npm run lintpasses).Summary by cubic
Adds unit tests for
parseProtocol,speedometer,throttle, andcomposeSignalsto cover critical paths and edge cases. Improves reliability around timeouts, throttling, and protocol parsing with no production code changes.Description
parseProtocol: http/https/ftp, + and -, data/file URIs, no protocol, 25-char limit.speedometer: default vs custom min, circular buffer wrap, numeric rate after min elapsed.throttle: immediate call, threshold enforcement, latest args after wait, flush, multi-arg passthrough.composeSignals: timeout abort with ETIMEDOUT reason, multiple signals, falsy filtering, unsubscribe support.Testing
tests/unit/helpers/; 36 tests added, all passing.npx vitest run tests/unit/helpers/.Written for commit 635af95. Summary will update on new commits.