Additional streams test consolidation gaps

[jasnell]

Filling more gaps

[ask-bonk]

A source error only takes the destination-abort branch; it never attempts to cancel the source. This test therefore cannot observe preventCancel, so a regression in that option passes while the comment claims both suppressions are covered. An abort signal initiates both shutdown actions.

Suggested change

	// preventAbort AND preventCancel together on a starts-errored source:
	// both suppressions hold and both ends stay un-shut-down.
	export const preventAbortAndCancelCombo = {
	async test() {
	const err = new Error('src-err');
	let abortCalled = false;
	const rs = new ReadableStream({
	start(c) {
	c.error(err);
	},
	});
	const ws = new WritableStream({
	abort() {
	abortCalled = true;
	},
	});
	strictEqual(
	await rejectionOf(
	rs.pipeTo(ws, {
	preventAbort: true,
	preventCancel: true,
	preventClose: true,
	})
	),
	err
	);
	strictEqual(abortCalled, false);
	ws.getWriter(); // dest untouched and re-lockable
	},
	};
	// An abort signal triggers both abort-destination and cancel-source
	// shutdown actions; preventAbort and preventCancel must suppress both.
	export const preventAbortAndCancelCombo = {
	async test() {
	const err = new Error('abort-reason');
	const abortController = new AbortController();
	let abortCalled = false;
	let cancelCalled = false;
	const rs = new ReadableStream({
	cancel() {
	cancelCalled = true;
	},
	});
	const ws = new WritableStream({
	abort() {
	abortCalled = true;
	},
	});
	const pipeP = rs.pipeTo(ws, {
	preventAbort: true,
	preventCancel: true,
	preventClose: true,
	signal: abortController.signal,
	});
	await scheduler.wait(1);
	abortController.abort(err);
	strictEqual(await rejectionOf(pipeP), err);
	strictEqual(abortCalled, false);
	strictEqual(cancelCalled, false);
	strictEqual(rs.locked, false);
	ws.getWriter(); // dest untouched and re-lockable
	},
	};

[ask-bonk]

This only observes that abort() has not run before the write is released. A broken pipe could reject before releaseWrite() while deferring the abort hook, and would still pass here. Track the pipe promise itself to verify that shutdown does not settle early.

Suggested change

	const pipeP = rs.pipeTo(ws);
	controller.enqueue('chunk');
	await scheduler.wait(10);
	controller.error(err);
	await scheduler.wait(20);
	// The write is still parked: abort must not have run yet.
	strictEqual(events.join(','), 'write-start');
	releaseWrite();
	strictEqual(await rejectionOf(pipeP), err);
	const pipeP = rs.pipeTo(ws);
	let pipeSettled = false;
	pipeP.then(
	() => (pipeSettled = true),
	() => (pipeSettled = true)
	);
	controller.enqueue('chunk');
	await scheduler.wait(10);
	controller.error(err);
	await scheduler.wait(20);
	// The write is still parked: the pipe and its abort action must not settle yet.
	strictEqual(events.join(','), 'write-start');
	strictEqual(pipeSettled, false);
	releaseWrite();
	strictEqual(await rejectionOf(pipeP), err);

[ask-bonk]

The comment says this pins the stream's final error, but the test only checks callback order. A regression that changes writer.closed's rejection reason would pass.

Suggested change

	export const abortThenControllerErrorInFlightWrite = {
	async test() {
	const events = [];
	let rejectWrite;
	let controller;
	const ws = new WritableStream({
	start(c) {
	controller = c;
	},
	write() {
	return new Promise((resolve, reject) => (rejectWrite = reject));
	},
	abort(reason) {
	events.push(`sink-abort:${reason}`);
	},
	});
	const writer = ws.getWriter();
	const write = writer.write('chunk');
	write.catch((e) => events.push(`write-rejected:${e.message}`));
	await scheduler.wait(1);
	const abortP = writer.abort('abort-reason');
	abortP.then(
	() => events.push('abort-fulfilled'),
	(e) => events.push(`abort-rejected:${e.message}`)
	);
	controller.error(new Error('controller-error'));
	rejectWrite(new Error('write-failure'));
	await scheduler.wait(20);
	// PARITY: both implementations run the sink's abort hook EAGERLY,
	// before the in-flight write settles, then surface the write
	// rejection, then fulfill the abort.
	strictEqual(
	events.join(' | '),
	'sink-abort:abort-reason | write-rejected:write-failure | abort-fulfilled'
	);
	},
	export const abortThenControllerErrorInFlightWrite = {
	async test() {
	const events = [];
	let rejectWrite;
	let controller;
	const ws = new WritableStream({
	start(c) {
	controller = c;
	},
	write() {
	return new Promise((resolve, reject) => (rejectWrite = reject));
	},
	abort(reason) {
	events.push(`sink-abort:${reason}`);
	},
	});
	const writer = ws.getWriter();
	const closed = writer.closed;
	const write = writer.write('chunk');
	write.catch((e) => events.push(`write-rejected:${e.message}`));
	await scheduler.wait(1);
	const abortP = writer.abort('abort-reason');
	abortP.then(
	() => events.push('abort-fulfilled'),
	(e) => events.push(`abort-rejected:${e.message}`)
	);
	controller.error(new Error('controller-error'));
	rejectWrite(new Error('write-failure'));
	await scheduler.wait(20);
	// PARITY: both implementations run the sink's abort hook EAGERLY,
	// before the in-flight write settles, then surface the write
	// rejection, then fulfill the abort.
	strictEqual(
	events.join(' | '),
	'sink-abort:abort-reason | write-rejected:write-failure | abort-fulfilled'
	);
	await rejects(closed, (e) => e === 'abort-reason');
	},
	};

[ask-bonk]

I'm Bonk, and I've done a quick review of your PR.

Adds streams regression coverage and suite documentation.

1. [P2] preventCancel is not exercised in error-propagation.js:285.
2. [P2] The in-flight-write test does not verify the pipe remains pending in error-propagation.js:340.
3. [P2] The writable abort test does not assert its documented final error in abort-semantics.js:492.

Posted three inline suggestion comments.

Time for a pun! These tests need to get their assertions in stream.

github run

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 37.28%. Comparing base (f7e36bf) to head (19350d6).

Additional details and impacted files

@@                          Coverage Diff                           @@
##           jasnell/streams-test-consolidation-10    #7182   +/-   ##
======================================================================
  Coverage                                  37.28%   37.28%           
======================================================================
  Files                                        800      800           
  Lines                                     251331   251331           
  Branches                                   19998    19998           
======================================================================
  Hits                                       93718    93718           
  Misses                                    146266   146266           
  Partials                                   11347    11347           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.