From 7c0f77499ce8d527c9ddbcdebd7edc6190000868 Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Sat, 12 Sep 2026 17:59:01 +0000 Subject: [PATCH] Address review on plugin install Rethrow errors other than ENOENT and ENOTDIR when reading the package spec as a directory, mark the tests that run npm as slow so FAST_TEST skips them, and use ./external_plugins/my_plugin in the README examples. Co-Authored-By: Claude Fable 5.1 --- packages/controller/README.md | 4 ++-- packages/ctl/README.md | 4 ++-- packages/host/README.md | 4 ++-- packages/lib/src/shared_commands.ts | 6 ++++-- test/lib/shared_commands.js | 9 +++++---- 5 files changed, 15 insertions(+), 12 deletions(-) diff --git a/packages/controller/README.md b/packages/controller/README.md index fc7490e5..8aaa9f90 100644 --- a/packages/controller/README.md +++ b/packages/controller/README.md @@ -44,9 +44,9 @@ Add plugin either by require path or relative/absolute path to the plugin direct A relative path must start with ./ or ../ (or .\ and ..\ on Windows) otherwise it will be assumed to be a require path for an installed package in node_modules. This is mainly useful for plugins under development, packages published on npm are better installed with `plugin install`. -For example, adding a plugin checked out next to the Clusterio installation: +For example, adding a plugin checked out in the external_plugins directory: - npx clusteriocontroller plugin add ../my_plugin + npx clusteriocontroller plugin add ./external_plugins/my_plugin #### `plugin remove ` diff --git a/packages/ctl/README.md b/packages/ctl/README.md index 7664313f..c116dd3b 100644 --- a/packages/ctl/README.md +++ b/packages/ctl/README.md @@ -46,9 +46,9 @@ Add plugin either by require path or relative/absolute path to the plugin direct A relative path must start with ./ or ../ (or .\ and ..\ on Windows) otherwise it will be assumed to be a require path for an installed package in node_modules. This is mainly useful for plugins under development, packages published on npm are better installed with `plugin install`. -For example, adding a plugin checked out next to the Clusterio installation: +For example, adding a plugin checked out in the external_plugins directory: - npx clusterioctl plugin add ../my_plugin + npx clusterioctl plugin add ./external_plugins/my_plugin #### `plugin remove ` diff --git a/packages/host/README.md b/packages/host/README.md index 6d02205a..071c459c 100644 --- a/packages/host/README.md +++ b/packages/host/README.md @@ -45,9 +45,9 @@ Add plugin either by require path or relative/absolute path to the plugin direct A relative path must start with ./ or ../ (or .\ and ..\ on Windows) otherwise it will be assumed to be a require path for an installed package in node_modules. This is mainly useful for plugins under development, packages published on npm are better installed with `plugin install`. -For example, adding a plugin checked out next to the Clusterio installation: +For example, adding a plugin checked out in the external_plugins directory: - npx clusteriohost plugin add ../my_plugin + npx clusteriohost plugin add ./external_plugins/my_plugin #### `plugin remove ` diff --git a/packages/lib/src/shared_commands.ts b/packages/lib/src/shared_commands.ts index 8c594037..397df07a 100644 --- a/packages/lib/src/shared_commands.ts +++ b/packages/lib/src/shared_commands.ts @@ -74,8 +74,10 @@ async function packageNameFromSpec(packageSpec: string) { if (typeof packageJson.name === "string") { return packageJson.name; } - } catch { - // Not a local directory, treat it as name@version + } catch (err: any) { + if (!["ENOENT", "ENOTDIR"].includes(err.code)) { + throw err; + } } return packageSpec.replace(/(?!^)@.*$/, ""); } diff --git a/test/lib/shared_commands.js b/test/lib/shared_commands.js index 83d3df8a..87e68472 100644 --- a/test/lib/shared_commands.js +++ b/test/lib/shared_commands.js @@ -4,6 +4,7 @@ const fs = require("fs/promises"); const path = require("path"); const lib = require("@clusterio/lib"); +const { slowTest } = require("../integration"); describe("lib/shared_commands", function() { @@ -71,7 +72,7 @@ describe("lib/shared_commands", function() { }); it("should install and add an npm plugin", async function() { - this.timeout(60000); + slowTest(this); await fs.writeFile("package.json", JSON.stringify({ name: "clusterio-install", private: true })); await run("install", testPluginPath); assert.equal(process.exitCode, undefined); @@ -82,7 +83,7 @@ describe("lib/shared_commands", function() { }); it("should report plugins that are already installed", async function() { - this.timeout(60000); + slowTest(this); await fs.writeFile("package.json", JSON.stringify({ name: "clusterio-install", private: true })); await run("install", testPluginPath); assert.deepEqual([...pluginList], [["test_plugin", "test_plugin"]]); @@ -94,7 +95,7 @@ describe("lib/shared_commands", function() { }); it("should fail when the package is not a plugin", async function() { - this.timeout(60000); + slowTest(this); await fs.writeFile("package.json", JSON.stringify({ name: "clusterio-install", private: true })); const notAPlugin = path.resolve("not-a-plugin"); await fs.mkdir(notAPlugin); @@ -105,7 +106,7 @@ describe("lib/shared_commands", function() { }); it("should fail when npm install fails", async function() { - this.timeout(60000); + slowTest(this); await fs.writeFile("package.json", JSON.stringify({ name: "clusterio-install", private: true })); await run("install", path.resolve("does-not-exist")); assert.equal(process.exitCode, 1);