magento / magento2-functional-testing-framework

Magento2 Functional Testing Framework
Other
155 stars 132 forks source link

MFTF-33780: Changed loose comparisons into strict #871

Closed bohdan-harniuk closed 3 years ago

bohdan-harniuk commented 3 years ago

Description

In this PR loose comparison operators were changed to the strict comparison operators. These changes apply to the next comparison operators:

  1. == changed to ===
  2. != changed to !==

Reason of those changes: With the PHP RFC updates often changes rules of loose comparison operators interpretation. So, for each MFTF codebase update to the new PHP version, if there is any update in the loose comparison operators interpretation, we must check every line of code where such operators are used to clarify if those changes applies to our codebase. A lot easier to replace loose operators with the strict ones to eliminate such problems in the future.

This PR doesn’t cover operations that perform non-strict comparisons with the next parts:

Those cases would be considered only if any build fails on them.

I've run all Magento 2 MFTF tests for the CE, EE, B2B repositories on the current branch:

Screenshot 2021-08-20 at 01 58 27 Screenshot 2021-08-20 at 01 57 36

All tests passed successfully:

Screenshot 2021-08-20 at 12 58 42

https://github.com/magento/magento2/pull/33866

Fixed Issues (if relevant)

  1. https://github.com/magento/magento2/issues/33780: [MFTF] Investigate and fix code affected by saner string to number comparisons #33780

Contribution checklist

bohdan-harniuk commented 3 years ago

Hello, @jilu1!

Please, proceed with the code review.

Thanks, Bohdan.

jilu1 commented 3 years ago

@bohdan-harniuk The builds are run on PHP 7, right? Let's also run on PHP 8 when the docker image is available.

jilu1 commented 3 years ago

@magento import pull request to https://github.com/magento-commerce/magento2-functional-testing-framework

bohdan-harniuk commented 3 years ago

@bohdan-harniuk The builds are run on PHP 7, right? Let's also run on PHP 8 when the docker image is available.

@jilu1 Yes, let’s do so!

Thanks, Bohdan

magento-engcom-team commented 3 years ago

@jilu1 the pull request successfully imported.

jilu1 commented 3 years ago

@bohdan-harniuk I tried to merge this PR before PHP 8 is available, but when I run on PHP 7.4, I found these 4 tests consistently fail for EE and B2B builds. The failure is not present when running with MFTF develop branch. Can you please double check?

Screen Shot 2021-08-25 at 9 28 11 AM
bohdan-harniuk commented 3 years ago

@bohdan-harniuk I tried to merge this PR before PHP 8 is available, but when I run on PHP 7.4, I found these 4 tests consistently fail for EE and B2B builds. The failure is not present when running with MFTF develop branch. Can you please double check?

Screen Shot 2021-08-25 at 9 28 11 AM

Hello, @jilu1!

You just need to rerun MFTF tests. There are few common issues with them. We see them quite often. Sorry for being late. I missed the letter about it in gmail.

Thanks, Bohdan

andrewbess commented 3 years ago

Hello @jilu1 FYI, currently, free API of currency converter is down

currency-converter-free-api-is-down

It will provoke an error in each functional test build

jilu1 commented 3 years ago

@andrewbess @bohdan-harniuk I have re-run builds later to decide if we can merge this PR now or wait for validation with PHP 8.

bohdan-harniuk commented 3 years ago

Hello, @jilu1!

For now free API of currency converter is up:

Screenshot 2021-09-03 at 10 09 16

Could you please rerun builds from your side? You can additionally recheck if it is UP by this link: server-status

Thanks, Bohdan