Skip to content

Commit d5415d5

Browse files
authored
Merge pull request #228 from stronk7/phpdoc_command_support_max_warnings
Add --max-warnings option to the PHPDoc command
2 parents 7a9b5b5 + 6da610a commit d5415d5

File tree

11 files changed

+220
-13
lines changed

11 files changed

+220
-13
lines changed

.travis.dist.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,11 @@ script:
5050
- moodle-plugin-ci phplint
5151
- moodle-plugin-ci phpcpd
5252
- moodle-plugin-ci phpmd
53-
- moodle-plugin-ci phpcs
53+
- moodle-plugin-ci phpcs --max-warnings 0
54+
- moodle-plugin-ci phpdoc --max-warnings 0
5455
- moodle-plugin-ci validate
5556
- moodle-plugin-ci savepoints
5657
- moodle-plugin-ci mustache
5758
- moodle-plugin-ci grunt
58-
- moodle-plugin-ci phpdoc --fail-on-warning
59-
- moodle-plugin-ci phpunit
59+
- moodle-plugin-ci phpunit --fail-on-warning
6060
- moodle-plugin-ci behat

docs/CHANGELOG.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@ The format of this change log follows the advice given at [Keep a CHANGELOG](htt
1010

1111
## [Unreleased]
1212
### Added
13-
- Add the `--testdox` option within the `phpunit` command.
13+
- Add the `--testdox` option to the `phpunit` command.
14+
- Add the `--max-warnings` option to the `phpdoc` command, so it behaves the same than the `phpcs` command. Note that this modifies current behaviour (see next point) and plugins with only warnings won't be failing any more.
15+
- ACTION SUGGESTED: In order to keep the previous behaviour, it's recommended to use the new `--max-warnings 0` (or any other number) option to specify the warnings threshold.
1416

1517
### Changed
1618
- Updated project dependencies to current [moodle-cs](https://github.com/moodlehq/moodle-cs), [moodle-local_moodlecheck](https://github.com/moodlehq/moodle-local_moodlecheck) and [moodle-local_ci](https://github.com/moodlehq/moodle-local_ci) versions. Also, to various internal / development tools.

docs/CLI.md

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1666,7 +1666,7 @@ Run Moodle PHPDoc Checker on a plugin
16661666

16671667
### Usage
16681668

1669-
* `phpdoc [-m|--moodle MOODLE] [--] <plugin>`
1669+
* `phpdoc [-m|--moodle MOODLE] [--max-warnings MAX-WARNINGS] [--] <plugin>`
16701670

16711671
Run Moodle PHPDoc Checker on a plugin
16721672

@@ -1692,6 +1692,16 @@ Path to Moodle
16921692
* Is negatable: no
16931693
* Default: `'.'`
16941694

1695+
#### `--max-warnings`
1696+
1697+
Number of warnings to trigger nonzero exit code - default: -1
1698+
1699+
* Accept value: yes
1700+
* Is value required: yes
1701+
* Is multiple: no
1702+
* Is negatable: no
1703+
* Default: `-1`
1704+
16951705
#### `--help|-h`
16961706

16971707
Display help for the given command. When no command is given display help for the list command

docs/GHAFileExplained.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ jobs:
150150

151151
- name: Moodle PHPDoc Checker
152152
if: ${{ always() }}
153-
run: moodle-plugin-ci phpdoc
153+
run: moodle-plugin-ci phpdoc --max-warnings 0
154154

155155
- name: Validating
156156
if: ${{ always() }}

docs/Help.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ you keep this step. To fail on warnings use `--max-warnings 0`
4545

4646
**moodle-plugin-ci phpdoc**
4747

48-
This step runs Moodle PHPDoc checker on your plugin.
48+
This step runs Moodle PHPDoc checker on your plugin. To fail on warnings use `--max-warnings 0`
4949

5050
**moodle-plugin-ci validate**
5151

docs/TravisFileExplained.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ script:
136136
# To fail on warnings use --max-warnings 0
137137
- moodle-plugin-ci phpcs
138138
# This step runs Moodle PHPDoc checker on your plugin.
139+
# To fail on warnings use --max-warnings 0
139140
- moodle-plugin-ci phpdoc
140141
# This step runs some light validation on the plugin file structure
141142
# and code. Validation can be plugin specific.

gha.dist.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ jobs:
8585

8686
- name: Moodle PHPDoc Checker
8787
if: ${{ always() }}
88-
run: moodle-plugin-ci phpdoc
88+
run: moodle-plugin-ci phpdoc --max-warnings 0
8989

9090
- name: Validating
9191
if: ${{ always() }}

src/Command/PHPDocCommand.php

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
use MoodlePluginCI\Bridge\MoodlePlugin;
1616
use Symfony\Component\Console\Input\InputInterface;
17+
use Symfony\Component\Console\Input\InputOption;
1718
use Symfony\Component\Console\Output\OutputInterface;
1819
use Symfony\Component\Filesystem\Filesystem;
1920
use Symfony\Component\Finder\Finder;
@@ -30,7 +31,9 @@ protected function configure(): void
3031
parent::configure();
3132

3233
$this->setName('phpdoc')
33-
->setDescription('Run Moodle PHPDoc Checker on a plugin');
34+
->setDescription('Run Moodle PHPDoc Checker on a plugin')
35+
->addOption('max-warnings', null, InputOption::VALUE_REQUIRED,
36+
'Number of warnings to trigger nonzero exit code - default: -1', -1);
3437
}
3538

3639
protected function initialize(InputInterface $input, OutputInterface $output): void
@@ -74,9 +77,19 @@ protected function execute(InputInterface $input, OutputInterface $output): int
7477
}
7578

7679
// moodlecheck.php does not return valid exit status,
77-
// We have to parse output to see if there are errors.
78-
$results = $process->getOutput();
80+
// We have to parse output to see if there are errors and/or warnings.
81+
$results = $process->getOutput();
82+
$totalProblems = (int) preg_match_all('~^\\s+Line~m', $results);
83+
$totalWarnings = (int) preg_match_all('~\(warning\)$~m', $results);
84+
$totalErrors = $totalProblems - $totalWarnings;
85+
$pseudoExitCode = ($totalErrors > 0) ? 1 : 0; // Calculate exit code based on # errors by default.
86+
87+
// If we aren't using the max-warnings option, (pseudo) process exit code is enough for us.
88+
if ($input->getOption('max-warnings') < 0) {
89+
return $pseudoExitCode;
90+
}
7991

80-
return (preg_match('/\s+Line/', $results)) ? 1 : 0;
92+
// With errors or warnings over the max-warnings threshold, fail the command.
93+
return ($totalErrors > 0 || ($totalWarnings > $input->getOption('max-warnings'))) ? 1 : 0;
8194
}
8295
}
Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
<?php
2+
3+
// This file is part of the Moodle Plugin CI package.
4+
//
5+
// Moodle is free software: you can redistribute it and/or modify
6+
// it under the terms of the GNU General Public License as published by
7+
// the Free Software Foundation, either version 3 of the License, or
8+
// (at your option) any later version.
9+
//
10+
// Moodle is distributed in the hope that it will be useful,
11+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
// GNU General Public License for more details.
14+
//
15+
// You should have received a copy of the GNU General Public License
16+
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
17+
18+
namespace MoodlePluginCI\Tests\Command;
19+
20+
use MoodlePluginCI\Bridge\MoodleConfig;
21+
use MoodlePluginCI\Command\PHPDocCommand;
22+
use MoodlePluginCI\Installer\Database\MySQLDatabase;
23+
use MoodlePluginCI\Tests\Fake\Bridge\DummyMoodlePHPDoc;
24+
use MoodlePluginCI\Tests\Fake\Bridge\DummyMoodlePlugin;
25+
use MoodlePluginCI\Tests\Fake\Process\DummyExecute;
26+
use MoodlePluginCI\Tests\MoodleTestCase;
27+
use Symfony\Component\Console\Application;
28+
use Symfony\Component\Console\Tester\CommandTester;
29+
30+
/**
31+
* Unit tests for PHPDocCommand.
32+
*
33+
* @copyright 2023 onwards Eloy Lafuente (stronk7) {@link https://stronk7.com}
34+
* @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
35+
*/
36+
class PHPDocCommandTest extends MoodleTestCase
37+
{
38+
protected function executeCommand($pluginDir = null, $maxWarnings = -1, $mockOutput = ''): CommandTester
39+
{
40+
if ($pluginDir === null) {
41+
$pluginDir = $this->pluginDir;
42+
}
43+
44+
$command = new PHPDocCommand();
45+
$command->moodle = new DummyMoodlePHPDoc($this->moodleDir);
46+
$command->plugin = new DummyMoodlePlugin($pluginDir);
47+
$command->execute = new DummyExecute(); // Mocked execution.
48+
$command->execute->returnOutput = $mockOutput; // Mocked output.
49+
50+
// Note: we leave this here as reference for any other command test needing a config.php file,
51+
// but it's not needed for this unit tess that is using a mocked execute() method.
52+
// Create a basic config.php file. This command requires it.
53+
// $config = new MoodleConfig();
54+
// $configContents = $config->createContents(new MySQLDatabase(), '/path/to/moodledata');
55+
// $config->dump($this->moodleDir . DIRECTORY_SEPARATOR . 'config.php', $configContents);
56+
57+
$application = new Application();
58+
$application->add($command);
59+
60+
$options = ['plugin' => $pluginDir];
61+
if ($maxWarnings >= 0) {
62+
$options['--max-warnings'] = $maxWarnings;
63+
}
64+
65+
$commandTester = new CommandTester($application->find('phpdoc'));
66+
$commandTester->execute($options);
67+
68+
return $commandTester;
69+
}
70+
71+
public function testExecute()
72+
{
73+
$commandTester = $this->executeCommand();
74+
$this->assertSame(0, $commandTester->getStatusCode());
75+
76+
// Verify various parts of the output.
77+
$output = $commandTester->getDisplay();
78+
79+
// Also verify display info is correct.
80+
$this->assertMatchesRegularExpression('/RUN Moodle PHPDoc Checker on local_ci/', $output);
81+
}
82+
83+
public function testExecuteFail()
84+
{
85+
$mockOutput = ' Line 12: Some error happened';
86+
$commandTester = $this->executeCommand($this->pluginDir, -1, $mockOutput);
87+
$this->assertSame(1, $commandTester->getStatusCode());
88+
89+
// Verify various parts of the output.
90+
// Note: Because this is a mocked process, with mocked output, we cannot inspect the command output
91+
// as we do in other tests. Only the title is available.
92+
// Some day all the mocks should be refactored to allow this.
93+
// $output = $commandTester->getDisplay();
94+
// $this->assertMatchesRegularExpression('xxxxxxx/', $output); // Progress.
95+
96+
// Also verify display info is correct.
97+
$this->assertMatchesRegularExpression('/RUN Moodle PHPDoc Checker/', $commandTester->getDisplay());
98+
}
99+
100+
public function testExecuteWithWarningsAndThreshold()
101+
{
102+
// Let's add a file with 2 warnings, and verify how the max-warnings affects the outcome.
103+
$mockOutput = <<<'EOT'
104+
Line 2: Empty line found after PHP open tag (warning)
105+
Line 12: Function someclass::somefunc is not documented (warning)
106+
107+
EOT;
108+
// By default it passes.
109+
$commandTester = $this->executeCommand($this->pluginDir, -1, $mockOutput);
110+
$this->assertSame(0, $commandTester->getStatusCode());
111+
112+
// Allowing 0 warning, it fails.
113+
$commandTester = $this->executeCommand($this->pluginDir, 0, $mockOutput);
114+
$this->assertSame(1, $commandTester->getStatusCode());
115+
116+
// Allowing 1 warning, it fails.
117+
$commandTester = $this->executeCommand($this->pluginDir, 1, $mockOutput);
118+
$this->assertSame(1, $commandTester->getStatusCode());
119+
120+
// Allowing 2 warnings, it passes.
121+
$commandTester = $this->executeCommand($this->pluginDir, 2, $mockOutput);
122+
$this->assertSame(0, $commandTester->getStatusCode());
123+
124+
// Allowing 3 warnings, it passes.
125+
$commandTester = $this->executeCommand($this->pluginDir, 3, $mockOutput);
126+
$this->assertSame(0, $commandTester->getStatusCode());
127+
}
128+
129+
public function testExecuteNoFiles()
130+
{
131+
// Just random directory with no PHP files.
132+
$commandTester = $this->executeCommand($this->pluginDir . '/tests/behat');
133+
$this->assertSame(0, $commandTester->getStatusCode());
134+
$this->assertMatchesRegularExpression('/No relevant files found to process, free pass!/', $commandTester->getDisplay());
135+
}
136+
137+
public function testExecuteNoPlugin()
138+
{
139+
$this->expectException(\InvalidArgumentException::class);
140+
$this->executeCommand('/path/to/no/plugin');
141+
}
142+
}
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<?php
2+
3+
// This file is part of the Moodle Plugin CI package.
4+
//
5+
// Moodle is free software: you can redistribute it and/or modify
6+
// it under the terms of the GNU General Public License as published by
7+
// the Free Software Foundation, either version 3 of the License, or
8+
// (at your option) any later version.
9+
//
10+
// Moodle is distributed in the hope that it will be useful,
11+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
// GNU General Public License for more details.
14+
//
15+
// You should have received a copy of the GNU General Public License
16+
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
17+
18+
namespace MoodlePluginCI\Tests\Fake\Bridge;
19+
20+
/**
21+
* Dummy Moodle Fixture class to be able to run PHPDocCommand tests.
22+
*
23+
* @copyright 2023 onwards Eloy Lafuente (stronk7) {@link https://stronk7.com}
24+
* @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
25+
*/
26+
class DummyMoodlePHPDoc extends DummyMoodle
27+
{
28+
public function normalizeComponent($component): array
29+
{
30+
return ['local', 'moodlecheck'];
31+
}
32+
33+
public function getComponentInstallDirectory($component): string
34+
{
35+
return $this->directory . '/local/moodlecheck';
36+
}
37+
}

0 commit comments

Comments
 (0)