Proposed solution for #5457 gherkin scenarios not loaded from group file #5458
Conversation
|
okey-dokey - looking at the output I can see at least a couple of tests that are failing because of my change. I'll address those and push up a commit. |
src/Codeception/Lib/GroupManager.php
Outdated
| @@ -118,6 +118,12 @@ public function groupsForTest(\PHPUnit\Framework\Test $test) | |||
| if (strpos($filename . ':' . $test->getName(false), $testPattern) === 0) { | |||
| $groups[] = $group; | |||
| } | |||
| // TODO: consider mb_strtolower per https://stackoverflow.com/a/5473569 | |||
| if (method_exists($test, 'getMetadata') | |||
| && strcasecmp($filename . ':' . $test->getMetadata()->getFeature(), $testPattern) === 0 | |||
There was a problem hiding this comment.
This needs to be UFT-8 compliant as put in your TODO
strcasecmp($filename . ':' . $test->getMetadata()->getFeature(), $testPattern) === 0to be replaced by mb_strtolower($filename . ':' . $test->getMetadata()->getFeature()) === mb_strtolower($testPattern)
src/Codeception/Lib/GroupManager.php
Outdated
| @@ -118,6 +118,12 @@ public function groupsForTest(\PHPUnit\Framework\Test $test) | |||
| if (strpos($filename . ':' . $test->getName(false), $testPattern) === 0) { | |||
| $groups[] = $group; | |||
| } | |||
| // TODO: consider mb_strtolower per https://stackoverflow.com/a/5473569 | |||
There was a problem hiding this comment.
Is there a case where metadata() method does not exist ?
There was a problem hiding this comment.
In some tests it did not so I added the check to make sure it was there before I called it
src/Codeception/Lib/GroupManager.php
Outdated
| @@ -118,6 +118,12 @@ public function groupsForTest(\PHPUnit\Framework\Test $test) | |||
| if (strpos($filename . ':' . $test->getName(false), $testPattern) === 0) { | |||
| $groups[] = $group; | |||
| } | |||
| // TODO: consider mb_strtolower per https://stackoverflow.com/a/5473569 | |||
| if (method_exists($test, 'getMetadata') | |||
| && strcasecmp($filename . ':' . $test->getMetadata()->getFeature(), $testPattern) === 0 | |||
There was a problem hiding this comment.
Since current solution is focused on Gherkin feature, before using $test->getMetadata()->getFeature(), you should verify that the current $test object is a Gherkin Test
There was a problem hiding this comment.
I can do that. Are there any other types of tests that might benefit from this comparison though?
|
@edno I still have the bigger question of - what tests should I write to verify this? I'm thinking I need to verify GroupManager so I've been digging into that. I would definitely appreciate any help you could give though. |
|
@edno actually, I think I figured it out. I've added two tests that fail before my changes to GroupManager but pass after my changes. Do you see any issues with them? Please note that the Japanese text I have used is supposed to be (at least according to Google) a translation of Jeff returns a faulty microwave. My apologies if it is not. |
|
I've not encountered this issue myself, but tests look good. |
|
Awesome! If this is all good, can we get this merged? My team is waiting for this change to run our automated tests. |
|
Not sure when PR will be merged. {
"repositories" : [
{
"type": "vcs",
"url": "https://github.com/mozillalives/Codeception/tree/fix-gherkin-scenario-run"
}
],
"require-dev": {
"codeception/codecetion":"2.5"
}
} |
|
Thank you! Looks good to me |
* Proposed solution for #5457 gherkin scenarios not loaded from group file (#5458) * proposed solution for #5457 gherkin scenarios not loaded from group file * testing for getMetaData method before calling it in GroupManager * reformatting if statement to appease nitpick * adding tests for #5457 * Update LOCAL_FILE constant for new version of phpseclib (#5461) * Using button formaction attr in proceedSubmitForm method (#5440) * Using button formaction attr in proceedSubmitForm method (AcceptanceTesterActions::submitForm) * Update InnerBrowser.php * Avoid removing required fields in cookies. (#5470)
A possible solution for the issue described in #5457. I have a couple of open questions though.
Codeception\Lib\GroupManagerthe best place to address this issue or is there a better place?strcasecmplooks like it will fail for multibyte strings. Are there suggestions on better alternatives? One I saw (that I linked to in a comment in the code) suggests usingmb_strtolowerinstead.Any help, tips or advice would be appreciated. Thanks.