Remove extract-zip from TypeScript test dependencies

[kpumuk]

The TypeScript browser test tooling still pulled extract-zip through grunt-contrib-qunit 3.1.0 and Puppeteer 1.x.

This updates grunt-contrib-qunit to 10.2.0, selects Puppeteer 25.8.0 for its browser runner, and removes extract-zip from the development dependency tree. It also aligns the downloaded QUnit assets to 2.26.0, whose event API is expected by the current Grunt plugin.

Puppeteer 25 requires Node.js 22.12 or newer for this development-only test path.

• Did you create an Apache Jira ticket? (Request account here, not required for trivial changes)
• If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
• Did you squash your changes to a single commit? (not required, but preferred)
• Did you do your best to avoid breaking changes? If one was needed, did you label the Jira ticket with "Breaking-Change"?
• If your change does not involve any code, include [skip ci] anywhere in the commit message to free up build resources.

[Copilot]

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-qunit to ^10.2.0 and pin its Puppeteer runner to 25.8.0 via overrides.
• Update QUnit assets downloaded for TS tests to 2.26.0 and switch those downloads to HTTPS.
• Regenerate lib/ts/package-lock.json to reflect the new dependency tree (removing extract-zip and 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.

[Jens-G]

Code review

No blocking issues found. Checked for bugs and CLAUDE.md compliance.

Two suggestions, below the bar for an issue but verified:

• This change needs a JIRA ticket. CONTRIBUTING.md: "All significant changes require an Apache Jira THRIFT Issue ticket. Trivial changes such as fixing a typo or a compiler warning do not." Moving grunt-contrib-qunit from 3.1 to 10.2 and Puppeteer from 1.20 to 25.8 changes about 55 package entries in the lockfile and the Node.js requirement of this test path. With a ticket, the title and the commit subject also get their THRIFT-NNNN: prefix; right now they don't match ("Remove extract-zip from TypeScript test dependencies" vs. "Update TypeScript QUnit dependencies").

thrift/CONTRIBUTING.md

Lines 15 to 17 in 5acac82

	1. All significant changes require an [Apache Jira THRIFT Issue](http://issues.apache.org/jira/browse/THRIFT) ticket. Trivial changes such as fixing a typo or a compiler warning do not.

• The Puppeteer override makes Node.js 22.12 the minimum for the lib/ts browser tests: puppeteer, puppeteer-core and @puppeteer/browsers all declare "node": ">=22.12.0" in the lockfile. Update JavaScript js-yaml dependencies #3724 refreshed the same lockfile and stated that it kept Node.js 16 compatibility. The PR description mentions the new floor, but nothing in the tree records it. Copilot's inline comment on this line is still unanswered.

thrift/lib/ts/package.json

Lines 39 to 43 in 5acac82

	"grunt-legacy-util": "2.0.2",
	"grunt-contrib-qunit": {
	"puppeteer": "25.8.0"
	},
	"jshint": {

🤖 Generated with Claude Code

[Copilot]

🔵 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/browsers dependency declares yauzl as an optional ZIP extractor, but this lockfile leaves that peer uninstalled. Its browser download path therefore falls back to a system unzip executable; make -C lib/ts check can fail during Puppeteer's install/download step on minimal environments without unzip. Add yauzl as a development dependency (and regenerate the lockfile), or make unzip an 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, while lib/ts is not included in lib/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

[Jens-G]

Ran this locally

I built the PR head (0fc68b1) and master (6b1d360) side by side in the same container —
Ubuntu 22.04 with ant/Java 17/unzip, Node v22.23.2, compiler built from each tree — to see what
holds up. Summary: everything in this PR that can be exercised today works, and the parts that
cannot be exercised cannot be exercised on master either.

npm install and the ant target are fine. Install on Node 22.23.2 exits 0, extract-zip is
gone from the tree, and ant download_jslibs fetches QUnit 2.26.0 over HTTPS with
BUILD SUCCESSFUL. npm audit goes from 13 findings on master (4 low, 9 high) to 9 here
(4 low, 5 high).

The new QUnit runner works. Against a minimal QUnit page served over HTTP — the same
mechanism Gruntfile.js uses — grunt-contrib-qunit 10.2 + Puppeteer 25.8 + QUnit 2.26.0 gives
2 tests completed, 0 failed, exit 0; with a deliberately failing assertion it gives 1 failed
and exit 6. The old stack behaves the same. So the plugin/QUnit event API pairing this PR is
about holds up. What I could not confirm is Thrift's own test.html under QUnit 2.26 — see
below for why.

npx grunt does not complete, on this branch or on master, for two reasons that predate this
PR. Both reproduce identically in both trees:

1. shell:BuildTS fails with test.ts(58,34) and test.ts(72,34) TS2345: Argument of type 'string' is not assignable to parameter of type 'Buffer<ArrayBufferLike>'. test.ts is
   byte-identical in both trees, and typescript (5.7.3) and @types/node (22.10.5) are the
   same in both lockfiles.
2. With that cast away locally, shell:BrowserifyCompiledTS then fails — browserify 16.2.3
   cannot parse the current uuid that the root npm install pulls in.

Filed as THRIFT-6317 and THRIFT-6318, plus THRIFT-6319 for the underlying reason nobody noticed:
lib/ts is in no SUBDIRS block and configure.ac has no AX_THRIFT_LIB(ts, …), so
check-local never runs, and no CI job runs it either.

One verified behaviour change worth a line in the PR. Copilot's note about yauzl is
correct, and it is a real difference rather than a theoretical one:

	without system unzip	with unzip
this branch (@puppeteer/browsers 3.2.1)	exit 1 — extraction fails, see below	exit 0
master (extract-zip, pure JS)	exit 0, Chromium extracted	—

Error: All providers failed for chrome-headless-shell 152.0.7977.42:
  - DefaultProvider: Extraction failed: no zip archiver is available. Install `unzip`
    (or `tar.exe`/Powershell on Windows), or add the optional `yauzl` dependency.

So the unzip dependency moves from npm into the operating system. Not a blocker — every CI image
here has unzip — but either adding yauzl as a devDependency or naming unzip as a
prerequisite would keep this path self-contained.

Two small things while you are in here: package.json still has no engines.node, though
puppeteer, puppeteer-core and @puppeteer/browsers all declare >=22.12.0; and
build.xml line 102 still fetches jQuery over plain HTTP while the four QUnit URLs right below
it now use HTTPS. I am fixing the same pattern in lib/js/test/build.xml, test/index.html,
test/features/index.html and tutorial/js/tutorial.html under THRIFT-6316 (#3899), and deliberately
leaving that one line to you so the two changes do not collide.

🤖 Generated with Claude Code