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();
	});
	});
	awaitpromise;
	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.