Skip to content

Conversation

orlangur
Copy link
Contributor

it shall not pass code review as object under test is mocked and thus it does not test anything.

Description

Fixed Issues (if relevant)

Improper test introduced in #13716

Manual testing scenarios

Not needed.

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/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

it shall not pass code review as object under test is mocked and thus it does not test anything
@magento-engcom-team
Copy link
Contributor

Hi @ihor-sviziev, thank you for the review.
ENGCOM-1105 has been created to process this Pull Request

@alepane21
Copy link
Contributor

Hi there @orlangur,
actually the check that I made was on method joinUrlRewrite that is not mocked, so it checks that the conditions passed to joinUrl uses the right store_id.

@orlangur
Copy link
Contributor Author

@alepane21 object under test shall not be mocked. In unit test "unit" is a whole object and not one method.

@alepane21
Copy link
Contributor

Ok, thanks for the explanation.
To have the pull request accepted I had to add a unit test and this was the fastest way I found to di it, sorry!

@orlangur
Copy link
Contributor Author

@alepane21 no problem 👍 You implemented an integration test also which fits perfectly for this type of change. Appreciate that!

@magento-engcom-team magento-engcom-team merged commit c1f39c8 into 2.2-develop Mar 29, 2018
@okorshenko okorshenko deleted the orlangur-patch-1 branch March 29, 2018 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants