Maintenance

[moonglum]

• Adjust Node support matrix (on main already)
• Remove mocha (use node:test)
• Remove minimist (use node:util)

[moonglum]

unskipping this test results in:

ℹ Error: Test "balks at invalid buckets" at test/test_plugins.js:234:2 generated asynchronous activity after the test ended. This activity created the error "AssertionError [ERR_ASSERTION]: Got unwanted rejection.
Actual message: "exit 1"" and would have caused the test to fail, but instead triggered an unhandledRejection event.
ℹ Error: Test "balks at invalid buckets" at test/test_plugins.js:234:2 generated asynchronous activity after the test ended. This activity created the error "AssertionError [ERR_ASSERTION]: Got unwanted rejection.
Actual message: "exit 1"" and would have caused the test to fail, but instead triggered an unhandledRejection event.
ℹ Error: Test "balks at invalid buckets" at test/test_plugins.js:234:2 generated asynchronous activity after the test ended. This activity created the error "AssertionError [ERR_ASSERTION]: Got unwanted rejection.
Actual message: "exit 1"" and would have caused the test to fail, but instead triggered an unhandledRejection event.
ℹ Error: Test "balks at invalid buckets" at test/test_plugins.js:234:2 generated asynchronous activity after the test ended. This activity created the error "AssertionError [ERR_ASSERTION]: Got unwanted rejection.
Actual message: "exit 1"" and would have caused the test to fail, but instead triggered an unhandledRejection event.

The code runs into the abort. This seems to lead to an unhandled promise rejection 🤔 Any idea, @FND ?

[cjihrig]

assert.rejects() and assert.doesNotReject() should be awaited I believe.

[moonglum]

Thank you, but that doesn't change the error.

[cjihrig]

It looks like the unhandledRejection events are present on the main branch, but not being reported anywhere. This diff seems to make them go away for me locally:

diff --git a/test/test_plugins.js b/test/test_plugins.js
index 104631d..808136e 100644
--- a/test/test_plugins.js
+++ b/test/test_plugins.js
@@ -1,6 +1,5 @@
 /* global describe, before, after, it */
 "use strict";
-
 let { pluginsByBucket, _determinePlugins } = require("../lib/plugins");
 let path = require("path");
 let assert = require("assert");
@@ -187,12 +186,12 @@ describe("plugin resolution", () => {
 		}));
 	});
 
-	it("balks at invalid package identifiers", () => {
-		assert.rejects(async () => {
+	it("balks at invalid package identifiers", async () => {
+		await assert.rejects(async () => {
 			return _determinePlugins(["faucet-pipeline-yummy"]);
 		}, /exit 1/);
 
-		assert.rejects(() => {
+		await assert.rejects(() => {
 			return _determinePlugins([{
 				// NB: local configuration must not be comprehensive to ensure
 				//     plugin is loaded
@@ -202,8 +201,8 @@ describe("plugin resolution", () => {
 		}, /exit 1/);
 	});
 
-	it("balks at duplicate configuration keys", () => {
-		assert.rejects(() => {
+	it("balks at duplicate configuration keys", async () => {
+		await assert.rejects(() => {
 			return _determinePlugins([{
 				key: "dummy",
 				bucket: "static",
@@ -216,34 +215,36 @@ describe("plugin resolution", () => {
 		}, /exit 1/);
 	});
 
-	it("balks at invalid plugins", () => {
-		assert.rejects(() => {
+	it("balks at invalid plugins", async () => {
+		await assert.rejects(() => {
 			return _determinePlugins(["faucet-pipeline-invalid-a"]);
 		}, /exit 1/);
 
-		assert.rejects(() => {
+		await assert.rejects(() => {
 			return _determinePlugins(["faucet-pipeline-invalid-b"]);
 		}, /exit 1/);
 
-		assert.rejects(() => {
+		await assert.rejects(() => {
 			return _determinePlugins(["faucet-pipeline-invalid-c"]);
 		}, /exit 1/);
 	});
 
-	it("balks at invalid buckets", () => {
+	it("balks at invalid buckets", async () => {
 		let plugin = {
 			key: "dummy",
 			plugin: () => {}
 		};
-		["static", "scripts", "styles", "markup"].forEach(bucket => {
+		const buckets = ["static", "scripts", "styles", "markup"];
+		for(let i = 0; i < buckets.length; ++i) {
+			const bucket = buckets[i];
 			plugin.bucket = bucket;
-			assert.doesNotReject(() => {
-				return _determinePlugins([plugin]);
+			await assert.doesNotReject(async () => {
+				await _determinePlugins([plugin]);
 			}, /exit 1/);
-		});
+		}
 
 		plugin.bucket = "dummy";
-		assert.rejects(() => {
+		await assert.rejects(() => {
 			return _determinePlugins([plugin]);
 		}, /exit 1/);
 	});

[moonglum]

Thanks a lot, @cjihrig 🙇 That's absolutely correct! I adjusted the code in a follow up commit that I will fixup into the original commit after @FND's review.

[FND]

Oh wow, thanks @cjihrig! I remember getting tripped up by that a few times in the past - so probably wouldn't have spotted this here right away...

Out of curiosity: Do you have some kind of alert for Node's test runner or how did you notice this obscure PR?

Either way, thanks for your work on that: It helps reduce suffering in the world.

[cjihrig]

Not an alert. I just happened to be doing a GitHub code search yesterday 😄

[FND]

Lucky us! 🙇

[FND]

Excellent work - thank you!

[FND]

This is delightful! If I'm not mistaken, this should get rid of a whopping 130 dependencies - except json5 and tsconfig-paths depend on minimist as well for some reason, so it's "just" 50 fewer dependencies.

On a related note, because both those packages appear to be transitive dependencies of eslint-config-fnd: I've recently decided not to bother upgrading the latter (which triggers deprecation warnings) because that's a major pita, plus that elaborate DIY style was never a good idea in the first place... 😬 So, uhm, apologies for sinking our ecosystem in favor of pixel perfection?

[moonglum]

So... you are now using deno lint && deno format instead of eslint-config-fnd?

[FND]

Correct. It's still a risk, but less so. (Of course diff pollution seems likely either way.)

[moonglum]

Tracked as #145