New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add CLI IgnoreViolationsOnExit option flag #380
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -200,7 +200,7 @@ public function __construct(array $args, array $availableRuleSets = array()) | |
case '--strict': | ||
$this->strict = true; | ||
break; | ||
case '--ignore-exit-violations': | ||
case '--ignore-violations-on-exit': | ||
$this->ignoreExitViolations = true; | ||
break; | ||
case (preg_match('(^\-\-reportfile\-(xml|html|text)$)', $arg, $match) > 0): | ||
|
@@ -338,11 +338,11 @@ public function hasStrict() | |
} | ||
|
||
/** | ||
* Was the <b>--ignore-exit-violations</b> passed to PHPMD's command line interface? | ||
* Was the <b>--ignore-violations-on-exit</b> passed to PHPMD's command line interface? | ||
* | ||
* @return boolean | ||
*/ | ||
public function hasIgnoreExitViolations() | ||
public function ignoreExitViolations() | ||
{ | ||
return $this->ignoreExitViolations; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rename to |
||
} | ||
|
@@ -459,8 +459,8 @@ public function usage() | |
'ignore directories' . \PHP_EOL . | ||
'--strict: also report those nodes with a @SuppressWarnings ' . | ||
'annotation' . \PHP_EOL . | ||
'--ignore-exit-violations: will exit with 0 even if a ' . | ||
'violation is found ' . \PHP_EOL; | ||
'--ignore-violations-on-exit: will exit with a zero code, ' . | ||
'even if any violations are found' . \PHP_EOL; | ||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -174,7 +174,7 @@ public function testThrowsExpectedExceptionWhenInputFileNotExists() | |
} | ||
|
||
/** | ||
* testCliOptionsAcceptsVersionArgument | ||
* testHasVersionReturnsFalseByDefault | ||
* | ||
* @return void | ||
*/ | ||
|
@@ -199,6 +199,45 @@ public function testCliOptionsAcceptsVersionArgument() | |
self::assertTrue($opts->hasVersion()); | ||
} | ||
|
||
/** | ||
* testIgnoreExitViolationsReturnsFalseByDefault | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Personally, I don't like it, when people just cpy and paste it. Why not say what it does, like: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought it was a Lead Developper choice so I just followed the testcase coding style There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is or better said was. But it's never too late to start improving 😼 |
||
* | ||
* @return void | ||
*/ | ||
public function testIgnoreExitViolationsReturnsFalseByDefault() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rename to |
||
{ | ||
$args = array(__FILE__, __FILE__, 'text', 'unusedcode'); | ||
$opts = new CommandLineOptions($args); | ||
|
||
self::assertFalse($opts->ignoreExitViolations()); | ||
} | ||
|
||
/** | ||
* testCliOptionsAcceptsIgnoreViolationsOnExitArgument | ||
* | ||
* @return void | ||
*/ | ||
public function testCliOptionsAcceptsIgnoreViolationsOnExitArgument() | ||
{ | ||
$args = array(__FILE__, __FILE__, 'text', 'unusedcode', '--ignore-violations-on-exit'); | ||
$opts = new CommandLineOptions($args); | ||
|
||
self::assertTrue($opts->ignoreExitViolations()); | ||
} | ||
|
||
/** | ||
* testCliUsageContainsStrictOption | ||
* | ||
* @return void | ||
*/ | ||
public function testCliUsageContainsIgnoreViolationsOnExitOption() | ||
{ | ||
$args = array(__FILE__, __FILE__, 'text', 'codesize'); | ||
$opts = new CommandLineOptions($args); | ||
|
||
$this->assertContains('--ignore-violations-on-exit:', $opts->usage()); | ||
} | ||
|
||
/** | ||
* testCliUsageContainsStrictOption | ||
* | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -99,4 +99,29 @@ public function testMainReturnsViolationExitCodeForSourceWithNPathViolation() | |
); | ||
$this->assertEquals(Command::EXIT_VIOLATION, $exitCode); | ||
} | ||
|
||
/** | ||
* testMainReturnsViolationExitCodeForSourceWithNPathViolation | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Copy paste error, better explain the meaning of the test instead of copy pasting the name of the function/method. This kind of documentation is meant for a developer, not for a computer... 😼 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought it was a Lead Developper choice so I just followed the testcase coding style |
||
* | ||
* @return void | ||
* @covers \PHPMD\TextUI\Command | ||
* @group phpmd | ||
* @group phpmd::textui | ||
* @group unittest | ||
*/ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a |
||
public function testMainReturnsSuccessExitCodeForSourceWithNPathViolationAndIgnoreViolationsOnExitFlag() | ||
{ | ||
$exitCode = Command::main( | ||
array( | ||
__FILE__, | ||
self::createFileUri('source/source_with_npath_violation.php'), | ||
'text', | ||
'codesize', | ||
'--reportfile', | ||
self::createTempFileUri(), | ||
'--ignore-violations-on-exit', | ||
) | ||
); | ||
$this->assertEquals(Command::EXIT_SUCCESS, $exitCode); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On a secont thought, after we renamed the CLI option, we should rename this appropriately, too.
Please rename to
ignoreViolationsOnExit()
.🙇