Skip to content

Update unit testing flows partial migration to vitest#7484

Merged
jasonsaayman merged 38 commits intov1.xfrom
feat/update-testing-flows
Mar 6, 2026
Merged

Update unit testing flows partial migration to vitest#7484
jasonsaayman merged 38 commits intov1.xfrom
feat/update-testing-flows

Conversation

@jasonsaayman
Copy link
Copy Markdown
Member

@jasonsaayman jasonsaayman commented Mar 6, 2026

Summary by cubic

Partially migrates unit tests to Vitest with unit and browser projects, adds a lightweight test server, and keeps Mocha/Karma running. Also transpiles the Node CJS bundle for Node 12.

Description

  • Summary of changes
    • Added Vitest config with two projects: unit (Node) and browser (Chromium via @vitest/browser-playwright).
    • Introduced tests/setup/server.js and a browser smoke test.
    • Migrated/added coverage for adapters (http, fetch, error details), Axios core, headers, helpers/utils, platform, transformResponse, prototype pollution, and combined regression suites.
    • Kept legacy Mocha/Karma tests; added minimal eslint/jasmine env hints only.
    • Updated package.json with Vitest scripts and deps; added Playwright. Rollup now transpiles the Node CJS bundle for Node 12 via @babel/preset-env.
    • Removed unneeded brainstorm docs.
  • Reasoning
    • Faster, modern test runner with cross-environment support while preserving current flows.
  • Additional context
    • No public API changes. Vitest tests are additive and can run alongside Mocha/Karma.

Docs

  • New scripts:
    • npm run test:vitest, npm run test:vitest:unit, npm run test:vitest:browser, npm run test:vitest:watch
  • One-time setup for browser tests: npx playwright install
  • Removed unneeded brainstorm docs; no new docs added.

Testing

  • Added tests (Vitest):
    • Adapters: tests/unit/adapters/http.test.js, fetch.test.js, error-details.test.js
    • Core/Headers: tests/unit/axios.test.js, axiosHeaders.test.js
    • Helpers/Utils: composeSignals, toFormData, fromDataURI, estimateDataURLDecodedBytes, parseProtocol, utils, platform
    • Regression: tests/unit/regression.test.js (combined issues + SSRF), prototypePollution.test.js, transformResponse.test.js
    • Browser: tests/browser/smoke.browser.test.js
    • Shared server utilities: tests/setup/server.js
  • Modified tests:
    • Legacy Mocha/Karma specs only received /* eslint-env mocha */ and jasmine hints; behavior unchanged.
  • Commands:
    • Unit: npm run test:vitest:unit
    • Browser: npm run test:vitest:browser (requires Playwright install)

Written for commit 12880b9. Summary will update on new commits.

@jasonsaayman jasonsaayman self-assigned this Mar 6, 2026
@jasonsaayman jasonsaayman added priority::medium A medium priority commit::test The PR is related to tests labels Mar 6, 2026
});

it('should support params', async () => {
server = await startHTTPServer((req, res) => res.end(req.url));

Check failure

Code scanning / CodeQL

Reflected cross-site scripting High test

Cross-site scripting vulnerability due to a
user-provided value
.

Copilot Autofix

AI 23 days ago

In general, to fix reflected XSS when echoing user-controlled values like req.url, you should escape or encode the data appropriately for the output context (HTML, attribute, JavaScript, etc.) before sending it in the HTTP response. In Node.js/Express-style code, a common approach is to use a well-known library such as escape-html to HTML-encode characters that could break out of the intended context (<, >, ", ', &, etc.).

For this specific test file, the minimal, non-functional change is to HTML-escape req.url before passing it to res.end. We can do this by importing the escape-html package at the top of tests/unit/adapters/fetch.test.js and then replacing res.end(req.url) on line 374 with res.end(escape(req.url)). This preserves the behavior of echoing back the URL text (now escaped), which is generally sufficient for a test that checks query parameter handling, while eliminating the direct reflection of raw user input. No other behavior in the tests needs to change.

Concretely:

  • Add import escape from 'escape-html'; alongside the other imports at the top of tests/unit/adapters/fetch.test.js.
  • Change the server handler in the "should support params" test from res.end(req.url) to res.end(escape(req.url)).
Suggested changeset 2
tests/unit/adapters/fetch.test.js

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/tests/unit/adapters/fetch.test.js b/tests/unit/adapters/fetch.test.js
--- a/tests/unit/adapters/fetch.test.js
+++ b/tests/unit/adapters/fetch.test.js
@@ -14,6 +14,7 @@
 import { AbortController } from 'abortcontroller-polyfill/dist/cjs-ponyfill.js';
 import util from 'util';
 import NodeFormData from 'form-data';
+import escape from 'escape-html';
 
 const pipelineAsync = util.promisify(stream.pipeline);
 
@@ -371,7 +372,7 @@
   });
 
   it('should support params', async () => {
-    server = await startHTTPServer((req, res) => res.end(req.url));
+    server = await startHTTPServer((req, res) => res.end(escape(req.url)));
 
     const { data } = await fetchAxios.get('/?test=1', {
       params: {
EOF
@@ -14,6 +14,7 @@
import { AbortController } from 'abortcontroller-polyfill/dist/cjs-ponyfill.js';
import util from 'util';
import NodeFormData from 'form-data';
import escape from 'escape-html';

const pipelineAsync = util.promisify(stream.pipeline);

@@ -371,7 +372,7 @@
});

it('should support params', async () => {
server = await startHTTPServer((req, res) => res.end(req.url));
server = await startHTTPServer((req, res) => res.end(escape(req.url)));

const { data } = await fetchAxios.get('/?test=1', {
params: {
package.json
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/package.json b/package.json
--- a/package.json
+++ b/package.json
@@ -175,7 +175,8 @@
   "dependencies": {
     "follow-redirects": "^1.15.11",
     "form-data": "^4.0.5",
-    "proxy-from-env": "^1.1.0"
+    "proxy-from-env": "^1.1.0",
+    "escape-html": "^1.0.3"
   },
   "bundlesize": [
     {
EOF
@@ -175,7 +175,8 @@
"dependencies": {
"follow-redirects": "^1.15.11",
"form-data": "^4.0.5",
"proxy-from-env": "^1.1.0"
"proxy-from-env": "^1.1.0",
"escape-html": "^1.0.3"
},
"bundlesize": [
{
This fix introduces these dependencies
Package Version Security advisories
escape-html (npm) 1.0.3 None
Copilot is powered by AI and may make mistakes. Always verify output.
it('should get response headers', async () => {
server = await startHTTPServer((req, res) => {
res.setHeader('foo', 'bar');
res.end(req.url);

Check failure

Code scanning / CodeQL

Reflected cross-site scripting High test

Cross-site scripting vulnerability due to a
user-provided value
.

Copilot Autofix

AI 23 days ago

In general, to prevent reflected XSS when sending user-controlled data (like req.url) in an HTTP response, you should apply appropriate output encoding for the context (HTML, attribute, JS, etc.), or otherwise ensure the data is safe (e.g., by validation or by returning it as a non-executable content type like text/plain with proper escaping).

For this specific test, the minimal, non‑functional change is to HTML-escape req.url before sending it. The test only checks that the exact string returned by the server matches a specific URL, so we also need to ensure that the test cases that depend on the raw URL either (a) compare against the encoded value, or (b) avoid echoing the URL at all when we don't need its exact value. Here, the params test ('should support params') asserts the raw URL string; we can avoid XSS in that test by ending the response with a constant safe string and only using req.url on the client side (the client already knows what URL it requested). For the 'should get response headers' test, the response body is irrelevant (we only inspect headers), so we can replace res.end(req.url) with a constant safe string. Concretely:

  • In the "support params" test, change the server callback from res.end(req.url); to res.end('OK'); and assert against something that does not require the server to echo the URL, e.g., assert on config.url or the request URL actually used (which axios exposes). However, we are constrained to only edit shown snippets and must not assume extra axios config fields beyond those already used. The simplest XSS-free change is to HTML-escape req.url and update the assertion to match that escaped string.
  • In the "get response headers" test, replace res.end(req.url); with res.end('OK'); because the body is unused in the test, removing the flow of user input into the response.

To implement this, within tests/unit/adapters/fetch.test.js we will:

  1. Add an import for a well-known escaping library such as escape-html.
  2. Use escape(req.url) instead of req.url where the echoed value is needed, and adjust the corresponding assertion to expect the escaped value.
  3. For the headers test where the body is irrelevant, change res.end(req.url); to res.end('OK');.
Suggested changeset 2
tests/unit/adapters/fetch.test.js

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/tests/unit/adapters/fetch.test.js b/tests/unit/adapters/fetch.test.js
--- a/tests/unit/adapters/fetch.test.js
+++ b/tests/unit/adapters/fetch.test.js
@@ -14,6 +14,7 @@
 import { AbortController } from 'abortcontroller-polyfill/dist/cjs-ponyfill.js';
 import util from 'util';
 import NodeFormData from 'form-data';
+import escape from 'escape-html';
 
 const pipelineAsync = util.promisify(stream.pipeline);
 
@@ -371,7 +372,7 @@
   });
 
   it('should support params', async () => {
-    server = await startHTTPServer((req, res) => res.end(req.url));
+    server = await startHTTPServer((req, res) => res.end(escape(req.url)));
 
     const { data } = await fetchAxios.get('/?test=1', {
       params: {
@@ -380,7 +381,7 @@
       },
     });
 
-    assert.strictEqual(data, '/?test=1&foo=1&bar=2');
+    assert.strictEqual(data, escape('/?test=1&foo=1&bar=2'));
   });
 
   it('should handle fetch failed error as an AxiosError with ERR_NETWORK code', async () => {
@@ -396,7 +397,7 @@
   it('should get response headers', async () => {
     server = await startHTTPServer((req, res) => {
       res.setHeader('foo', 'bar');
-      res.end(req.url);
+      res.end('OK');
     });
 
     const { headers } = await fetchAxios.get('/', {
EOF
@@ -14,6 +14,7 @@
import { AbortController } from 'abortcontroller-polyfill/dist/cjs-ponyfill.js';
import util from 'util';
import NodeFormData from 'form-data';
import escape from 'escape-html';

const pipelineAsync = util.promisify(stream.pipeline);

@@ -371,7 +372,7 @@
});

it('should support params', async () => {
server = await startHTTPServer((req, res) => res.end(req.url));
server = await startHTTPServer((req, res) => res.end(escape(req.url)));

const { data } = await fetchAxios.get('/?test=1', {
params: {
@@ -380,7 +381,7 @@
},
});

assert.strictEqual(data, '/?test=1&foo=1&bar=2');
assert.strictEqual(data, escape('/?test=1&foo=1&bar=2'));
});

it('should handle fetch failed error as an AxiosError with ERR_NETWORK code', async () => {
@@ -396,7 +397,7 @@
it('should get response headers', async () => {
server = await startHTTPServer((req, res) => {
res.setHeader('foo', 'bar');
res.end(req.url);
res.end('OK');
});

const { headers } = await fetchAxios.get('/', {
package.json
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/package.json b/package.json
--- a/package.json
+++ b/package.json
@@ -175,7 +175,8 @@
   "dependencies": {
     "follow-redirects": "^1.15.11",
     "form-data": "^4.0.5",
-    "proxy-from-env": "^1.1.0"
+    "proxy-from-env": "^1.1.0",
+    "escape-html": "^1.0.3"
   },
   "bundlesize": [
     {
EOF
@@ -175,7 +175,8 @@
"dependencies": {
"follow-redirects": "^1.15.11",
"form-data": "^4.0.5",
"proxy-from-env": "^1.1.0"
"proxy-from-env": "^1.1.0",
"escape-html": "^1.0.3"
},
"bundlesize": [
{
This fix introduces these dependencies
Package Version Security advisories
escape-html (npm) 1.0.3 None
Copilot is powered by AI and may make mistakes. Always verify output.
return;
}

Promise.resolve(handler(req, res)).then((result) => {
@jasonsaayman jasonsaayman force-pushed the feat/update-testing-flows branch from 4f39f14 to 12880b9 Compare March 6, 2026 18:16
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 91 files

Confidence score: 4/5

  • This PR looks safe to merge overall: both findings are low severity (4/10) and mainly affect test coverage/portability rather than core runtime behavior.
  • The main risk is in tests/unit/adapters/fetch.test.js, where using the default axios instance means fetch-adapter basic auth behavior is not actually validated, so a fetch-specific regression could slip through.
  • The issue in .cursor/plans/rewrite_helper_tests_a2f815ee.plan.md is housekeeping-oriented but worth fixing, since absolute local paths are machine-specific and reduce repo portability.
  • Pay close attention to tests/unit/adapters/fetch.test.js and .cursor/plans/rewrite_helper_tests_a2f815ee.plan.md - ensure the fetch adapter is truly exercised and replace absolute paths with repo-relative ones.

Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.

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/adapters/fetch.test.js">

<violation number="1" location="tests/unit/adapters/fetch.test.js:284">
P2: This test uses the default axios instance, so it doesn't exercise the fetch adapter and won't catch fetch-specific basic auth regressions. Use the fetchAxios instance here.</violation>
</file>

<file name=".cursor/plans/rewrite_helper_tests_a2f815ee.plan.md">

<violation number="1" location=".cursor/plans/rewrite_helper_tests_a2f815ee.plan.md:27">
P2: Avoid committing absolute local file paths; they are machine-specific and break portability. Use repo-relative paths instead.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

@jasonsaayman jasonsaayman merged commit fa33733 into v1.x Mar 6, 2026
10 of 11 checks passed
@jasonsaayman jasonsaayman deleted the feat/update-testing-flows branch March 6, 2026 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commit::test The PR is related to tests priority::medium A medium priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant