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

Make the module list more deterministic #21020

Merged
merged 2 commits into from Feb 24, 2019
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -126,24 +126,29 @@ private function getModuleConfigs()
*
* @param array $origList
* @return array
* @SuppressWarnings(PHPMD.UnusedLocalVariable)
* @throws \Exception
*/
private function sortBySequence($origList)
private function sortBySequence(array $origList): array
{
ksort($origList);
$modules = $this->prearrangeModules($origList);
$expanded = [];
foreach ($origList as $moduleName => $value) {
foreach (array_keys($modules) as $moduleName) {
$sequence = $this->expandSequence($origList, $moduleName);
asort($sequence);

This comment has been minimized.

Copy link
@orlangur

orlangur Feb 25, 2019

Contributor

It would make more sense to add this call into

$result = array_unique(array_merge($result, $relatedResult));
$expanded[] = [
'name' => $moduleName,
'sequence' => $this->expandSequence($origList, $moduleName),
'sequence' => $sequence,
];
}
// Use "bubble sorting" because usort does not check each pair of elements and in this case it is important
$total = count($expanded);
for ($i = 0; $i < $total - 1; $i++) {
for ($j = $i; $j < $total; $j++) {
if (in_array($expanded[$j]['name'], $expanded[$i]['sequence'])) {
if (in_array($expanded[$j]['name'], $expanded[$i]['sequence'], true)) {
$temp = $expanded[$i];
$expanded[$i] = $expanded[$j];
$expanded[$j] = $temp;
@@ -159,6 +164,27 @@ private function sortBySequence($origList)
return $result;
}
/**
* Prearrange all modules by putting those from Magento before the others
*
* @param array $modules
* @return array
*/
private function prearrangeModules(array $modules): array
{
$breakdown = ['magento' => [], 'others' => []];
foreach ($modules as $moduleName => $moduleDetails) {
if (strpos($moduleName, 'Magento_') !== false) {
$breakdown['magento'][$moduleName] = $moduleDetails;
} else {
$breakdown['others'][$moduleName] = $moduleDetails;
}
}
return array_merge($breakdown['magento'], $breakdown['others']);
}
/**
* Accumulate information about all transitive "sequence" references
*
@@ -160,4 +160,55 @@ public function testLoadCircular()
]));
$this->loader->load();
}
/**
* @throws \Magento\Framework\Exception\LocalizedException
*/
public function testLoadPrearranged(): void
{
$fixtures = [
'Foo_Bar' => ['name' => 'Foo_Bar', 'sequence' => ['Magento_Store']],
'Magento_Directory' => ['name' => 'Magento_Directory', 'sequence' => ['Magento_Store']],
'Magento_Store' => ['name' => 'Magento_Store', 'sequence' => []],
'Magento_Theme' => ['name' => 'Magento_Theme', 'sequence' => ['Magento_Store', 'Magento_Directory']],
'Test_HelloWorld' => ['name' => 'Test_HelloWorld', 'sequence' => ['Magento_Theme']]
];
$index = 0;
foreach ($fixtures as $name => $fixture) {
$this->converter->expects($this->at($index++))->method('convert')->willReturn([$name => $fixture]);
}
$this->registry->expects($this->once())
->method('getPaths')
->willReturn([
'/path/to/Foo_Bar',
'/path/to/Magento_Directory',
'/path/to/Magento_Store',
'/path/to/Magento_Theme',
'/path/to/Test_HelloWorld'
]);
$this->driver->expects($this->exactly(5))
->method('fileGetContents')
->will($this->returnValueMap([
['/path/to/Foo_Bar/etc/module.xml', null, null, self::$sampleXml],
['/path/to/Magento_Directory/etc/module.xml', null, null, self::$sampleXml],
['/path/to/Magento_Store/etc/module.xml', null, null, self::$sampleXml],
['/path/to/Magento_Theme/etc/module.xml', null, null, self::$sampleXml],
['/path/to/Test_HelloWorld/etc/module.xml', null, null, self::$sampleXml],
]));
// Load the full module list information
$result = $this->loader->load();
$this->assertSame(
['Magento_Store', 'Magento_Directory', 'Magento_Theme', 'Foo_Bar', 'Test_HelloWorld'],
array_keys($result)
);
foreach ($fixtures as $name => $fixture) {
$this->assertSame($fixture, $result[$name]);
}
}
}
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.