Skip to content

MFTF deprecation notice attributes #560

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

Merged
merged 9 commits into from
Jan 27, 2020
Merged

MFTF deprecation notice attributes #560

merged 9 commits into from
Jan 27, 2020

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

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.

Small changes requested around constants, logic seems good otherwise.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.5%) to 51.926% when pulling 3e86d4f on DeprecatedAttr into e344c6f on MFTF-2.5.5-RC.

@coveralls
Copy link

coveralls commented Jan 27, 2020

Coverage Status

Coverage increased (+0.6%) to 52.497% when pulling a457211 on DeprecatedAttr into 53b055d on develop.

@okolesnyk okolesnyk requested a review from KevinBKozan January 27, 2020 17:57
 - add verification test
@KevinBKozan
Copy link
Contributor

@okolesnyk Feature is still printing out all deprecated entities on generation; we need to either:

  • Suppress printing selectively for our new deprecation notices (done by changing function in MftfLoggingUtil)
  • Detect existance of deprecated entities once and only print out a single Deprecated entities detected in MFTF Test files, please check <log file>. This is trickier, would need to keep track of if we've thrown that error at all.

Option one is easier, and works to reduce noise. I think that once we implement this, the deprecated warning will literally always be up once it starts being used, so I actually think we can supress all deprecated test file warnings.

$sectionDeprecated = $sectionData[self::OBJ_DEPRECATED] ?? null;

if ($sectionDeprecated !== null) {
LoggingUtil::getInstance()->getLogger(ElementObject::class)->deprecation(
Copy link
Contributor

Choose a reason for hiding this comment

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

Missed on first review, should be SectionObject::class I believe.

KevinBKozan
KevinBKozan previously approved these changes Jan 27, 2020
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.

LGTM, verification tests look thorough, not concerned about manual test requirements.

@okolesnyk okolesnyk dismissed KevinBKozan’s stale review January 27, 2020 20:45

The base branch was changed.

@okolesnyk okolesnyk changed the base branch from MFTF-2.5.5-RC to develop January 27, 2020 20:45
@okolesnyk okolesnyk merged commit a84ee26 into develop Jan 27, 2020
@okolesnyk okolesnyk deleted the DeprecatedAttr branch April 30, 2020 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants