-
Notifications
You must be signed in to change notification settings - Fork 51
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
waitFor return check since waitForText doesn't assert #146
Open
smmccabe
wants to merge
1
commit into
pfrenssen:8.3.x
Choose a base branch
from
smmccabe:3252220-wait-for
base: 8.3.x
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
83 changes: 83 additions & 0 deletions
83
coder_sniffer/DrupalPractice/Sniffs/FunctionCalls/WaitForTextSniff.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
<?php | ||
/** | ||
* \DrupalPractice\Sniffs\FunctionCalls\WaitForText | ||
* | ||
* @category PHP | ||
* @package PHP_CodeSniffer | ||
* @link http://pear.php.net/package/PHP_CodeSniffer | ||
*/ | ||
|
||
namespace DrupalPractice\Sniffs\FunctionCalls; | ||
|
||
use PHP_CodeSniffer\Files\File; | ||
use Drupal\Sniffs\Semantics\FunctionCall; | ||
|
||
/** | ||
* Check that waitForText return values are always handled. | ||
* | ||
* @category PHP | ||
* @package PHP_CodeSniffer | ||
* @link http://pear.php.net/package/PHP_CodeSniffer | ||
*/ | ||
class WaitForTextSniff extends FunctionCall | ||
{ | ||
/** | ||
* Whether method invocations with the same function name should be processed, | ||
* too. | ||
* | ||
* @var boolean | ||
*/ | ||
protected $includeMethodCalls = true; | ||
|
||
|
||
/** | ||
* Returns an array of function names this test wants to listen for. | ||
* | ||
* @return array<string> | ||
*/ | ||
public function registerFunctionNames() | ||
{ | ||
return ['waitForText']; | ||
|
||
}//end registerFunctionNames() | ||
|
||
|
||
/** | ||
* Processes this function call. | ||
* | ||
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned. | ||
* @param int $stackPtr The position of the function call in | ||
* the stack. | ||
* @param int $openBracket The position of the opening | ||
* parenthesis in the stack. | ||
* @param int $closeBracket The position of the closing | ||
* parenthesis in the stack. | ||
* | ||
* @return void|int | ||
*/ | ||
public function processFunctionCall( | ||
File $phpcsFile, | ||
$stackPtr, | ||
$openBracket, | ||
$closeBracket | ||
) { | ||
$tokens = $phpcsFile->getTokens(); | ||
|
||
$start = $phpcsFile->findStartOfStatement($stackPtr); | ||
$end = $phpcsFile->findEndOfStatement($start); | ||
// We are assigning to a variable, all is well. | ||
if ($tokens[$start]['code'] === T_VARIABLE && $phpcsFile->findNext(T_EQUAL, $start, $end) !== false) { | ||
return; | ||
} | ||
|
||
$function = ($start - 2); | ||
if ($tokens[$function]['content'] === 'assertTrue' || $tokens[$function]['content'] === 'assertFalse') { | ||
return; | ||
} | ||
|
||
$phpcsFile->addWarning('waitForText functions do not self assert and must be asserted manually', $start, 'WaitForText'); | ||
|
||
}//end processFunctionCall() | ||
|
||
|
||
}//end class |
15 changes: 15 additions & 0 deletions
15
tests/DrupalPractice/FunctionCalls/WaitForTextUnitTest.inc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
<?php | ||
|
||
// Correct assertion | ||
$this->assertTrue($this->assertSession()->waitForText('Example Text')); | ||
$this->assertFalse($this->assertSession()->waitForText('Example Text')); | ||
|
||
// Assigned to a variable which we assume will be asserted later | ||
$wait = $this->assertSession()->waitForText('Example Text'); | ||
|
||
// Incorrect assertion, waitForText only returns true or false and should be asserted as such | ||
$this->assertEmpty($this->assertSession()->waitForText('Example Text')); | ||
|
||
// No Assertion, failure | ||
$this->assertSession()->waitForText('Example Text'); | ||
|
57 changes: 57 additions & 0 deletions
57
tests/DrupalPractice/FunctionCalls/WaitForTextUnitTest.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
<?php | ||
/** | ||
* Unit test class for the DbQuery sniff. | ||
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 leftover? |
||
*/ | ||
|
||
namespace DrupalPractice\Test\FunctionCalls; | ||
|
||
use Drupal\Test\CoderSniffUnitTest; | ||
|
||
/** | ||
* Unit test class for the WaitForText sniff. | ||
* | ||
* A sniff unit test checks a .inc file for expected violations of a single | ||
* coding standard. Expected errors and warnings are stored in this class. | ||
*/ | ||
class WaitForTextUnitTest extends CoderSniffUnitTest | ||
{ | ||
|
||
|
||
/** | ||
* Returns the lines where errors should occur. | ||
* | ||
* The key of the array should represent the line number and the value | ||
* should represent the number of errors that should occur on that line. | ||
* | ||
* @param string $testFile The name of the file being tested. | ||
* | ||
* @return array<int, int> | ||
*/ | ||
protected function getErrorList(string $testFile): array | ||
{ | ||
return []; | ||
|
||
}//end getErrorList() | ||
|
||
|
||
/** | ||
* Returns the lines where warnings should occur. | ||
* | ||
* The key of the array should represent the line number and the value | ||
* should represent the number of warnings that should occur on that line. | ||
* | ||
* @param string $testFile The name of the file being tested. | ||
* | ||
* @return array<int, int> | ||
*/ | ||
protected function getWarningList(string $testFile): array | ||
{ | ||
return [ | ||
11 => 1, | ||
14 => 1, | ||
]; | ||
|
||
}//end getWarningList() | ||
|
||
|
||
}//end class |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Should we add more of the waitFor* methods?