Skip to content

MQE-659: [ALLURE] Include the test "stepKey" in the MFTF Allure Report #359

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 6 commits into from
Closed

MQE-659: [ALLURE] Include the test "stepKey" in the MFTF Allure Report #359

wants to merge 6 commits into from

Conversation

okolesnyk
Copy link
Member

Description

Fixed Issues (if relevant)

  1. magento/magento2-functional-testing-framework#<issue_number>: Issue title
  2. ...

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/verification tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)
  • Changes to Framework doesn't have backward incompatible changes for tests or have related Pull Request with fixes to tests

@okolesnyk okolesnyk mentioned this pull request Jun 5, 2019
Closed
5 tasks
@KevinBKozan KevinBKozan self-requested a review June 5, 2019 19:25
KevinBKozan
KevinBKozan suggested changes Jun 5, 2019
View reviewed changes
Copy link
Contributor

@KevinBKozan KevinBKozan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doc changes + some actionGroup functionality is required. Works well with normal tests + suites.

Also I will create a branch off of this branch with the one-line stepKey investigation (looks promising)

src/Magento/FunctionalTestingFramework/Allure/Adapter/MagentoAllureAdapter.php
@@ -116,6 +120,11 @@ public function stepBefore(StepEvent $stepEvent)
{
//Hard set to 200; we don't expose this config in MFTF
$argumentsLength = 200;
$stepKey = null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docblock on ln 113 details what this stepBefore does, add inserts stepKey into step name to it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still need docblock update I believe

@okolesnyk
Copy link
Member Author

Hi @KevinBKozan
Please take a look at my latest changes.

@magento magento deleted a comment from coveralls Jun 6, 2019
@magento magento deleted a comment from coveralls Jun 6, 2019
@okolesnyk
Copy link
Member Author

with latest commit I fixed static test failure

@okolesnyk okolesnyk force-pushed the MQE-659-alex-edition branch from bf34ab1 to 4074560 Compare June 7, 2019 17:57
KevinBKozan
KevinBKozan suggested changes Jun 7, 2019
View reviewed changes
src/Magento/FunctionalTestingFramework/Allure/Adapter/MagentoAllureAdapter.php Outdated
$this->testFiles[$filePath] = explode(PHP_EOL, file_get_contents($filePath));
}

preg_match("/\/\/ stepKey: (?<stepKey>.*)/", $this->testFiles[$filePath][$stepLine], $matches);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not use preg_match, and instead use sscanf() so we can make use of the new const in TestGenerator.
This would require splitting the string to only the //stepKey: portion and utilizing two consts in TestGenerator instead

This way we don't need to keep two very similar constants in disparate classes.

See https://github.com/magento/magento2-functional-testing-framework/compare/MQE-659-oneline for the example.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you still want to use preg_match, at the very least move this regex to a const in eithe rthis class or TestGenerator

src/Magento/FunctionalTestingFramework/Allure/Adapter/MagentoAllureAdapter.php Outdated
// if actionGroup flag, start nesting
if (strpos($step->getName(), ActionGroupObject::ACTION_GROUP_CONTEXT_START) !== false) {
$step->setName(str_replace(ActionGroupObject::ACTION_GROUP_CONTEXT_START, '', $step->getName()));
$actionGroupStepContainer = $step;

preg_match('/\[(?<actionGroupStepKey>.*)\]/', $step->getName(), $matches);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this regex to a const in either actionGroupObject or this class.

src/Magento/FunctionalTestingFramework/Allure/Adapter/MagentoAllureAdapter.php
@@ -116,6 +120,11 @@ public function stepBefore(StepEvent $stepEvent)
{
//Hard set to 200; we don't expose this config in MFTF
$argumentsLength = 200;
$stepKey = null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still need docblock update I believe

@okolesnyk okolesnyk requested a review from KevinBKozan June 7, 2019 18:43
@okolesnyk
Copy link
Member Author

Hey @KevinBKozan
Moved everything into 1 PR to reduce confusion and make life easier.

@okolesnyk okolesnyk closed this Jun 7, 2019
@jilu1 jilu1 deleted the MQE-659-alex-edition branch February 6, 2020 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KevinBKozan KevinBKozan Awaiting requested review from KevinBKozan

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@okolesnyk @KevinBKozan