test: improve zlib tests

[anonrig]

Remove several unnecessary zlib tests, and use node:test as much as we can. This also avoids calling require('../common') functionality as much as we can.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.40%. Comparing base (5d4fee8) to head (c1b6cb1).
Report is 554 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #55716      +/-   ##
==========================================
- Coverage   88.42%   88.40%   -0.02%     
==========================================
  Files         654      654              
  Lines      187662   187763     +101     
  Branches    36118    36127       +9     
==========================================
+ Hits       165945   165999      +54     
- Misses      14955    15006      +51     
+ Partials     6762     6758       -4     

see 80 files with indirect coverage changes

[lpinca]

I'm fine with removing redundant tests but I'm -1 on using node:test as much as we can as per #54796 and #55110 (review). In my opinion this is not an improvement, but added complexity and spurious code executed as part of the test, which could also affect the reliability of the test itself.

[LiviaMedeiros]

I love how Promise.withResolvers() helps making callback tests relatively concise, but I don't see how this is better than using common.mustSucceed():

Suggested change

	const { promise, resolve } = Promise.withResolvers();
	zlib.deflate(inputString, (err, buffer) => {
	assert.ifError(err);
	zlib.inflate(buffer, (err, inflated) => {
	assert.ifError(err);
	assert.strictEqual(inflated.toString(), inputString);
	resolve();
	});
	});
	await promise;
	zlib.deflate(inputString, common.mustSucceed((buffer) => {
	zlib.inflate(buffer, common.mustSucceed((inflated) => {
	assert.strictEqual(inflated.toString(), inputString);
	}));
	}));

Is there a reason why we should avoid relying on common utilities?

[anonrig]

common.mustSucceed() depends on process.on('exit') hook, whereas Promise.withResolvers() doesn't.

[targos]

can't we await once(out, 'end') ?

[anonrig]

Yes, definitely! Nice catch. Thanks @targos

[lpinca]

This means replacing a Node.js specific API (common.mustCall()) with another Node.js specific API (events.once()). What's the point?

[anonrig]

common.mustCall() depends on process.on('exit') where we definitely don't need to wait for the process to exit to check the state for this current implementation.

[lpinca]

Ok but I don't see how it is an improvement. It doesn't change anything for the sake of the test.