From a1fce6a1bca25315c9b875c624ce01408bc61ae7 Mon Sep 17 00:00:00 2001 From: Philipp Memmel Date: Thu, 7 Mar 2024 06:25:55 +0000 Subject: [PATCH 1/2] Add ignore-npm-dependencies setting --- docs/CLI.md | 16 +++++++++++++--- src/Command/InstallCommand.php | 4 +++- src/Installer/InstallerFactory.php | 3 ++- src/Installer/VendorInstaller.php | 7 ++++--- 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/docs/CLI.md b/docs/CLI.md index 5fedbd7d..163b7fc7 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -332,7 +332,7 @@ Number of times to rerun failures Selenium Docker image * Accept value: yes -* Is value required: no +* Is value required: yes * Is multiple: no * Is negatable: no * Default: `NULL` @@ -859,7 +859,7 @@ Install everything required for CI testing ### Usage -* `install [--moodle MOODLE] [--data DATA] [--repo REPO] [--branch BRANCH] [--plugin PLUGIN] [--db-type DB-TYPE] [--db-user DB-USER] [--db-pass DB-PASS] [--db-name DB-NAME] [--db-host DB-HOST] [--db-port DB-PORT] [--not-paths NOT-PATHS] [--not-names NOT-NAMES] [--extra-plugins EXTRA-PLUGINS] [--no-init] [--node-version NODE-VERSION]` +* `install [--moodle MOODLE] [--data DATA] [--repo REPO] [--branch BRANCH] [--plugin PLUGIN] [--db-type DB-TYPE] [--db-user DB-USER] [--db-pass DB-PASS] [--db-name DB-NAME] [--db-host DB-HOST] [--db-port DB-PORT] [--not-paths NOT-PATHS] [--not-names NOT-NAMES] [--extra-plugins EXTRA-PLUGINS] [--no-init] [--node-version NODE-VERSION] [--ignore-npm-dependencies]` Install everything required for CI testing @@ -1025,6 +1025,16 @@ Node.js version to use for nvm install (this will override one defined in .nvmrc * Is negatable: no * Default: `NULL` +#### `--ignore-npm-dependencies` + +Does not execute npm install for the plugin + +* Accept value: no +* Is value required: no +* Is multiple: no +* Is negatable: no +* Default: `false` + #### `--help|-h` Display help for the given command. When no command is given display help for the list command @@ -2395,4 +2405,4 @@ Do not ask any interactive question * Is value required: no * Is multiple: no * Is negatable: no -* Default: `false` +* Default: `false` \ No newline at end of file diff --git a/src/Command/InstallCommand.php b/src/Command/InstallCommand.php index 69c40de2..2adb3203 100644 --- a/src/Command/InstallCommand.php +++ b/src/Command/InstallCommand.php @@ -102,7 +102,8 @@ protected function configure(): void ->addOption('not-names', null, InputOption::VALUE_REQUIRED, 'CSV of file names to exclude', $names) ->addOption('extra-plugins', null, InputOption::VALUE_REQUIRED, 'Directory of extra plugins to install', $extra) ->addOption('no-init', null, InputOption::VALUE_NONE, 'Prevent PHPUnit and Behat initialization') - ->addOption('node-version', null, InputOption::VALUE_REQUIRED, 'Node.js version to use for nvm install (this will override one defined in .nvmrc)', $node); + ->addOption('node-version', null, InputOption::VALUE_REQUIRED, 'Node.js version to use for nvm install (this will override one defined in .nvmrc)', $node) + ->addOption('ignore-npm-dependencies', null, InputOption::VALUE_NONE, 'Does not execute npm install for the plugin'); } protected function initialize(InputInterface $input, OutputInterface $output): void @@ -175,6 +176,7 @@ public function initializeInstallerFactory(InputInterface $input): InstallerFact $factory->pluginsDir = $pluginsDir; $factory->noInit = $input->getOption('no-init'); $factory->nodeVer = $input->getOption('node-version'); + $factory->ignoreNpmDependencies = $input->getOption('ignore-npm-dependencies'); $factory->database = $resolver->resolveDatabase( $input->getOption('db-type'), $input->getOption('db-name'), diff --git a/src/Installer/InstallerFactory.php b/src/Installer/InstallerFactory.php index cdb77ad7..f02ad47a 100644 --- a/src/Installer/InstallerFactory.php +++ b/src/Installer/InstallerFactory.php @@ -34,6 +34,7 @@ class InstallerFactory public ?string $pluginsDir; public bool $noInit; public ?string $nodeVer; + public ?string $ignoreNpmDependencies; /** * Given a big bag of install options, add installers to the collection. @@ -51,7 +52,7 @@ public function addInstallers(InstallerCollection $installers): void } $installers->add(new PluginInstaller($this->moodle, $this->plugin, $this->pluginsDir, $this->dumper)); - $installers->add(new VendorInstaller($this->moodle, $this->plugin, $this->execute, $this->nodeVer)); + $installers->add(new VendorInstaller($this->moodle, $this->plugin, $this->execute, $this->nodeVer, $this->ignoreNpmDependencies)); if ($this->noInit) { return; diff --git a/src/Installer/VendorInstaller.php b/src/Installer/VendorInstaller.php index 062c18f5..3f864076 100644 --- a/src/Installer/VendorInstaller.php +++ b/src/Installer/VendorInstaller.php @@ -26,7 +26,7 @@ class VendorInstaller extends AbstractInstaller private MoodlePlugin $plugin; private Execute $execute; public ?string $nodeVer; - + private ?string $ignoreNpmDependencies; /** * Define legacy Node version to use when .nvmrc is absent (Moodle < 3.5). */ @@ -38,12 +38,13 @@ class VendorInstaller extends AbstractInstaller * @param Execute $execute * @param string|null $nodeVer */ - public function __construct(Moodle $moodle, MoodlePlugin $plugin, Execute $execute, ?string $nodeVer) + public function __construct(Moodle $moodle, MoodlePlugin $plugin, Execute $execute, ?string $nodeVer, ?string $ignoreNpmDependencies) { $this->moodle = $moodle; $this->plugin = $plugin; $this->execute = $execute; $this->nodeVer = $nodeVer; + $this->ignoreNpmDependencies = $ignoreNpmDependencies; } public function install(): void @@ -69,7 +70,7 @@ public function install(): void $this->execute->mustRun( Process::fromShellCommandline('npm install --no-progress', $this->moodle->directory, null, null, null) ); - if ($this->plugin->hasNodeDependencies()) { + if (is_null($this->ignoreNpmDependencies) && $this->plugin->hasNodeDependencies()) { $this->execute->mustRun( Process::fromShellCommandline('npm install --no-progress', $this->plugin->directory, null, null, null) ); From c2ce78ddb82788442ca3e718dda7c5061c96ddc3 Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Thu, 7 Mar 2024 16:43:51 +0100 Subject: [PATCH 2/2] Rename option to no-plugin-node plus unit tests --- docs/CLI.md | 22 +++++++-------- src/Command/InstallCommand.php | 30 ++++++++++---------- src/Installer/InstallerFactory.php | 4 +-- src/Installer/VendorInstaller.php | 23 ++++++++------- tests/Fixture/plugin/package.json | 7 +++++ tests/Installer/VendorInstallerTest.php | 37 +++++++++++++++++++++++++ 6 files changed, 85 insertions(+), 38 deletions(-) create mode 100644 tests/Fixture/plugin/package.json diff --git a/docs/CLI.md b/docs/CLI.md index 163b7fc7..6e3d5f2f 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -859,7 +859,7 @@ Install everything required for CI testing ### Usage -* `install [--moodle MOODLE] [--data DATA] [--repo REPO] [--branch BRANCH] [--plugin PLUGIN] [--db-type DB-TYPE] [--db-user DB-USER] [--db-pass DB-PASS] [--db-name DB-NAME] [--db-host DB-HOST] [--db-port DB-PORT] [--not-paths NOT-PATHS] [--not-names NOT-NAMES] [--extra-plugins EXTRA-PLUGINS] [--no-init] [--node-version NODE-VERSION] [--ignore-npm-dependencies]` +* `install [--moodle MOODLE] [--data DATA] [--repo REPO] [--branch BRANCH] [--plugin PLUGIN] [--db-type DB-TYPE] [--db-user DB-USER] [--db-pass DB-PASS] [--db-name DB-NAME] [--db-host DB-HOST] [--db-port DB-PORT] [--not-paths NOT-PATHS] [--not-names NOT-NAMES] [--extra-plugins EXTRA-PLUGINS] [--no-init] [--no-plugin-node] [--node-version NODE-VERSION]` Install everything required for CI testing @@ -1015,25 +1015,25 @@ Prevent PHPUnit and Behat initialization * Is negatable: no * Default: `false` -#### `--node-version` +#### `--no-plugin-node` -Node.js version to use for nvm install (this will override one defined in .nvmrc) +Prevent Node.js plugin dependencies installation -* Accept value: yes -* Is value required: yes +* Accept value: no +* Is value required: no * Is multiple: no * Is negatable: no -* Default: `NULL` +* Default: `false` -#### `--ignore-npm-dependencies` +#### `--node-version` -Does not execute npm install for the plugin +Node.js version to use for nvm install (this will override one defined in .nvmrc) -* Accept value: no -* Is value required: no +* Accept value: yes +* Is value required: yes * Is multiple: no * Is negatable: no -* Default: `false` +* Default: `NULL` #### `--help|-h` diff --git a/src/Command/InstallCommand.php b/src/Command/InstallCommand.php index 2adb3203..827bf0ed 100644 --- a/src/Command/InstallCommand.php +++ b/src/Command/InstallCommand.php @@ -102,8 +102,8 @@ protected function configure(): void ->addOption('not-names', null, InputOption::VALUE_REQUIRED, 'CSV of file names to exclude', $names) ->addOption('extra-plugins', null, InputOption::VALUE_REQUIRED, 'Directory of extra plugins to install', $extra) ->addOption('no-init', null, InputOption::VALUE_NONE, 'Prevent PHPUnit and Behat initialization') - ->addOption('node-version', null, InputOption::VALUE_REQUIRED, 'Node.js version to use for nvm install (this will override one defined in .nvmrc)', $node) - ->addOption('ignore-npm-dependencies', null, InputOption::VALUE_NONE, 'Does not execute npm install for the plugin'); + ->addOption('no-plugin-node', null, InputOption::VALUE_NONE, 'Prevent Node.js plugin dependencies installation') + ->addOption('node-version', null, InputOption::VALUE_REQUIRED, 'Node.js version to use for nvm install (this will override one defined in .nvmrc)', $node); } protected function initialize(InputInterface $input, OutputInterface $output): void @@ -165,19 +165,19 @@ public function initializeInstallerFactory(InputInterface $input): InstallerFact $pluginsDir = realpath($validate->directory($pluginsDir)); } - $factory = new InstallerFactory(); - $factory->moodle = new Moodle($input->getOption('moodle')); - $factory->plugin = new MoodlePlugin($pluginDir); - $factory->execute = $this->execute; - $factory->repo = $validate->gitUrl($input->getOption('repo')); - $factory->branch = $validate->gitBranch($input->getOption('branch')); - $factory->dataDir = $input->getOption('data'); - $factory->dumper = $this->initializePluginConfigDumper($input); - $factory->pluginsDir = $pluginsDir; - $factory->noInit = $input->getOption('no-init'); - $factory->nodeVer = $input->getOption('node-version'); - $factory->ignoreNpmDependencies = $input->getOption('ignore-npm-dependencies'); - $factory->database = $resolver->resolveDatabase( + $factory = new InstallerFactory(); + $factory->moodle = new Moodle($input->getOption('moodle')); + $factory->plugin = new MoodlePlugin($pluginDir); + $factory->execute = $this->execute; + $factory->repo = $validate->gitUrl($input->getOption('repo')); + $factory->branch = $validate->gitBranch($input->getOption('branch')); + $factory->dataDir = $input->getOption('data'); + $factory->dumper = $this->initializePluginConfigDumper($input); + $factory->pluginsDir = $pluginsDir; + $factory->noInit = $input->getOption('no-init'); + $factory->noPluginNode = $input->getOption('no-plugin-node'); + $factory->nodeVer = $input->getOption('node-version'); + $factory->database = $resolver->resolveDatabase( $input->getOption('db-type'), $input->getOption('db-name'), $input->getOption('db-user'), diff --git a/src/Installer/InstallerFactory.php b/src/Installer/InstallerFactory.php index f02ad47a..5421a356 100644 --- a/src/Installer/InstallerFactory.php +++ b/src/Installer/InstallerFactory.php @@ -33,8 +33,8 @@ class InstallerFactory public ConfigDumper $dumper; public ?string $pluginsDir; public bool $noInit; + public bool $noPluginNode; public ?string $nodeVer; - public ?string $ignoreNpmDependencies; /** * Given a big bag of install options, add installers to the collection. @@ -52,7 +52,7 @@ public function addInstallers(InstallerCollection $installers): void } $installers->add(new PluginInstaller($this->moodle, $this->plugin, $this->pluginsDir, $this->dumper)); - $installers->add(new VendorInstaller($this->moodle, $this->plugin, $this->execute, $this->nodeVer, $this->ignoreNpmDependencies)); + $installers->add(new VendorInstaller($this->moodle, $this->plugin, $this->execute, $this->noPluginNode, $this->nodeVer)); if ($this->noInit) { return; diff --git a/src/Installer/VendorInstaller.php b/src/Installer/VendorInstaller.php index 3f864076..3f1fb3bc 100644 --- a/src/Installer/VendorInstaller.php +++ b/src/Installer/VendorInstaller.php @@ -25,8 +25,8 @@ class VendorInstaller extends AbstractInstaller private Moodle $moodle; private MoodlePlugin $plugin; private Execute $execute; + private bool $noPluginNode; public ?string $nodeVer; - private ?string $ignoreNpmDependencies; /** * Define legacy Node version to use when .nvmrc is absent (Moodle < 3.5). */ @@ -38,13 +38,13 @@ class VendorInstaller extends AbstractInstaller * @param Execute $execute * @param string|null $nodeVer */ - public function __construct(Moodle $moodle, MoodlePlugin $plugin, Execute $execute, ?string $nodeVer, ?string $ignoreNpmDependencies) + public function __construct(Moodle $moodle, MoodlePlugin $plugin, Execute $execute, bool $noPluginNode, ?string $nodeVer) { - $this->moodle = $moodle; - $this->plugin = $plugin; - $this->execute = $execute; - $this->nodeVer = $nodeVer; - $this->ignoreNpmDependencies = $ignoreNpmDependencies; + $this->moodle = $moodle; + $this->plugin = $plugin; + $this->execute = $execute; + $this->nodeVer = $nodeVer; + $this->noPluginNode = $noPluginNode; } public function install(): void @@ -65,12 +65,13 @@ public function install(): void $this->execute->mustRunAll($processes); - $this->getOutput()->step('Install npm dependencies'); + $this->getOutput()->step('Install Moodle npm dependencies'); $this->execute->mustRun( Process::fromShellCommandline('npm install --no-progress', $this->moodle->directory, null, null, null) ); - if (is_null($this->ignoreNpmDependencies) && $this->plugin->hasNodeDependencies()) { + if (!$this->noPluginNode && $this->plugin->hasNodeDependencies()) { + $this->getOutput()->step('Install plugin npm dependencies'); $this->execute->mustRun( Process::fromShellCommandline('npm install --no-progress', $this->plugin->directory, null, null, null) ); @@ -83,7 +84,9 @@ public function install(): void public function stepCount(): int { - return ($this->canInstallNode()) ? 3 : 2; + return 2 + // Normally 2 steps: global dependencies and Moodle npm dependencies. + ($this->canInstallNode() ? 1 : 0) + // Plus Node.js installation. + ((!$this->noPluginNode && $this->plugin->hasNodeDependencies()) ? 1 : 0); // Plus plugin npm dependencies step. } /** diff --git a/tests/Fixture/plugin/package.json b/tests/Fixture/plugin/package.json new file mode 100644 index 00000000..69e88c7d --- /dev/null +++ b/tests/Fixture/plugin/package.json @@ -0,0 +1,7 @@ +{ + "name": "VendorInstallerFixture", + "description": "", + "version": "0.1.0", + "dependencies": {}, + "devDependencies": {} +} \ No newline at end of file diff --git a/tests/Installer/VendorInstallerTest.php b/tests/Installer/VendorInstallerTest.php index b494b476..432715da 100644 --- a/tests/Installer/VendorInstallerTest.php +++ b/tests/Installer/VendorInstallerTest.php @@ -26,6 +26,7 @@ public function testInstall() new DummyMoodle($this->moodleDir), new MoodlePlugin($this->pluginDir), new DummyExecute(), + false, null ); $installer->install(); @@ -39,6 +40,7 @@ public function testInstallNodeNoNvmrc() new DummyMoodle($this->moodleDir), new MoodlePlugin($this->pluginDir), new DummyExecute(), + false, null ); @@ -58,6 +60,7 @@ public function testInstallNodeUserVersion() new DummyMoodle($this->moodleDir), new MoodlePlugin($this->pluginDir), new DummyExecute(), + false, $userVersion ); $installer->installNode(); @@ -66,4 +69,38 @@ public function testInstallNodeUserVersion() $this->assertTrue(is_file($this->moodleDir . '/.nvmrc')); $this->assertSame($userVersion, file_get_contents($this->moodleDir . '/.nvmrc')); } + + public function testInstallNodePluginDependencies() + { + $installer = new VendorInstaller( + new DummyMoodle($this->moodleDir), + new MoodlePlugin($this->pluginDir), + new DummyExecute(), + false, + null + ); + + $this->fs->copy(__DIR__ . '/../Fixture/plugin/package.json', $this->pluginDir . '/package.json'); + + $installer->install(); + + $this->assertSame(4, $installer->getOutput()->getStepCount()); + } + + public function testSkipNodePluginDependencies() + { + $installer = new VendorInstaller( + new DummyMoodle($this->moodleDir), + new MoodlePlugin($this->pluginDir), + new DummyExecute(), + true, + null + ); + + $this->fs->copy(__DIR__ . '/../Fixture/plugin/package.json', $this->pluginDir . '/package.json'); + + $installer->install(); + + $this->assertSame(3, $installer->getOutput()->getStepCount()); + } }