Repository navigation
THRIFT-6429: Update TypeScript browser-test dependencies - #3747
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the TypeScript browser test toolchain in lib/ts to remove the extract-zip dependency by upgrading the Grunt QUnit runner stack and aligning the downloaded QUnit assets with the plugin’s expected event API.
Changes:
- Bump
grunt-contrib-qunitto^10.2.0and pin its Puppeteer runner to25.8.0viaoverrides. - Update QUnit assets downloaded for TS tests to
2.26.0and switch those downloads to HTTPS. - Regenerate
lib/ts/package-lock.jsonto reflect the new dependency tree (removingextract-zipand older Puppeteer-era deps).
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| lib/ts/test/build.xml | Updates downloaded QUnit asset versions/URLs used by the TS test build. |
| lib/ts/package.json | Upgrades grunt-contrib-qunit and adds an override to select Puppeteer 25.8.0. |
| lib/ts/package-lock.json | Updates the resolved dependency graph to match the new dev dependency set (removing extract-zip). |
Files not reviewed (1)
- lib/ts/package-lock.json: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| "grunt-contrib-qunit": { | ||
| "puppeteer": "25.8.0" | ||
| }, |
Code reviewNo blocking issues found. Checked for bugs and CLAUDE.md compliance. Two suggestions, below the bar for an issue but verified:
Lines 15 to 17 in 5acac82
Lines 39 to 43 in 5acac82 🤖 Generated with Claude Code |
5acac82 to
0fc68b1
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
Unresolved dependency-installation and CI-validation issues remain.
Review details
Files not reviewed (1)
- lib/ts/package-lock.json: Generated file
Suppressed comments (2)
lib/ts/package-lock.json:58
- The new
@puppeteer/browsersdependency declaresyauzlas an optional ZIP extractor, but this lockfile leaves that peer uninstalled. Its browser download path therefore falls back to a systemunzipexecutable;make -C lib/ts checkcan fail during Puppeteer's install/download step on minimal environments withoutunzip. Addyauzlas a development dependency (and regenerate the lockfile), or makeunzipan explicit prerequisite for this test path.
lib/ts/package.json:22 - This major tooling update is not exercised by the repository's automated tests: the build workflow's TypeScript step runs
make -C lib/nodets check, whilelib/tsis not included inlib/Makefile.am. Please add a CI invocation for this Grunt/Puppeteer path (with Node >=22.12) or an equivalent validation before relying on the new runner and QUnit assets.
"grunt-contrib-qunit": "^10.2.0",
- Files reviewed: 2/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
Ran this locallyI built the PR head (0fc68b1) and master (6b1d360) side by side in the same container —
The new QUnit runner works. Against a minimal QUnit page served over HTTP — the same
Filed as THRIFT-6317 and THRIFT-6318, plus THRIFT-6319 for the underlying reason nobody noticed: One verified behaviour change worth a line in the PR. Copilot's note about
So the unzip dependency moves from npm into the operating system. Not a blocker — every CI image Two small things while you are in here: 🤖 Generated with Claude Code |
0fc68b1 to
33af63b
Compare
Client: js,ts Co-Authored-By: OpenAI Codex (gpt-6.1-sol) <codex@openai.com>
33af63b to
16b7857
Compare
|
Thanks for the investigation, @Jens-G. I’ve rebased onto master to pick up the separate TypeScript compilation, Browserify, and CI fixes, and updated this PR:
Compilation, lint, and all 26 browser tests passed on Node 22.12.0. I also reproduced the missing-extractor failure and verified that fresh ZIP-fixture installation succeeds with |

The TypeScript browser tests currently pull extract-zip 1.7.0 through grunt-contrib-qunit 3.1.0 and Puppeteer 1.20.0.
This updates grunt-contrib-qunit to 11 and uses its declared Puppeteer 25 dependency without an override. The lockfile selects Puppeteer 25.8.0 and no longer includes extract-zip. The downloaded QUnit assets move to 2.26.0 to match the plugin's event API, and all browser-test library downloads use HTTPS. Adding yauzl preserves browser ZIP installation when system unzip is unavailable.
The TypeScript test tools now require Node.js 22.12.0 or newer. This is declared in lib/ts/package.json, documented under the README's 0.26.0 breaking changes, and supported by selecting Node.js 22 in the TypeScript CI job. This does not change Node.js support for the Thrift runtime.
[skip ci]anywhere in the commit message to free up build resources.