Skip to content
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

Fix integration test expectation to match count of countries in the database #5095

Merged
merged 2 commits into from
Aug 12, 2016
Merged

Fix integration test expectation to match count of countries in the database #5095

merged 2 commits into from
Aug 12, 2016

Conversation

Vinai
Copy link
Contributor

@Vinai Vinai commented Jun 20, 2016

This PR applies to the 2.1.0-rc3 and develop branches.
It probably already is fixed upstream though since it is a simple test failure due to a hardcoded expectation that actually is a value from the database.

Details:
The number of countries in the database seems to have changed.
The test \Magento\Sales\Block\Adminhtml\Order\Create\Form\AddressTest::testGetForm() hardcoded the expected number of options in the country select to 246.
The number of records created in that table by \Magento\Directory\Setup\InstallData::install() is 245.
This PR adjusts the test so it expects the number of options in the select to match the number countries in the system country directory.

@Vinai
Copy link
Contributor Author

Vinai commented Jun 20, 2016

Strange - locally the original test fails because it expects 246 (hardcoded), but the country select only contains 245 options, so my change made it pass.
But in the travis build there is an extra option in the select option list that isn't there locally.
Where is the option list built?
What causes the difference?

@Vinai
Copy link
Contributor Author

Vinai commented Jun 20, 2016

I can't look into travis to see what happens there, but at least I can look how the local option count ends up being 245:

The country options list is created by \Magento\Directory\Model\ResourceModel\Country\Collection::toOptionArray().
The collection contains 245 items. One for each record in the database.
The following loop filters out countries with an empty country name for the locale.

foreach ($options as $data) {
    $name = (string)$this->_localeLists->getCountryTranslation($data['value']);
    if (!empty($name)) {
        $sort[$name] = $data['value'];
    }
}

In the local build, $name is empty for Andorra ($data['name] == 'AN');
After that loop the count of countries is down to 244.

Then, at the end of the toOptionArray() method, an empty option is prepended:

if (count($options) > 0 && $emptyLabel !== false) {
    array_unshift($options, ['value' => '', 'label' => $emptyLabel]);
}

This brings the option back up to 245.

My guess is that on the travis build the localized country name returned by $this->_localeLists->getCountryTranslation($data['value']); is not null.
Locally the countries \ResourceBundle for the en_US locale doesn't seem to contain that country name. The question is, why? And should the test fail if that is the case?

The address form actually is being built correctly, so I think not. The issue should probably be covered by a test for \Magento\Framework\Locale\TranslatedLists::getCountryTranslation().

@Vinai
Copy link
Contributor Author

Vinai commented Jun 20, 2016

Okay, this version of the test passes locally and on travis. The PR reduces the scope of the test to just check the country options get rendered in the form. Accepting this PR would help keeping other CI pipelines working out of the box.

@NadiyaS
Copy link
Contributor

NadiyaS commented Jun 27, 2016

Hi @Vinai ,
thank you for your contribution. Internal ticket MAGETWO-54783 was created to process your PR.

@NadiyaS NadiyaS added the Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development label Jun 27, 2016
@vkorotun vkorotun added Component: Sales bug report bugfix and removed CS Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development bug report labels Aug 3, 2016
@sshrewz sshrewz added the linked label Aug 11, 2016
@mmansoor-magento mmansoor-magento merged commit 395c230 into magento:develop Aug 12, 2016
mmansoor-magento pushed a commit that referenced this pull request Aug 12, 2016
@vkorotun vkorotun added Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development and removed linked labels Aug 22, 2016
magento-engcom-team pushed a commit that referenced this pull request Dec 12, 2019
[Performance] Random failure of PAT builds
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Component: Sales Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Progress: accept
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants