Fix bug where imported matrix parameter duplicates are not overrided (#…

…1461) When importing a matrix from another matrix, the intended behavior is that any duplicate parameter keys favor the value in the matrix doing the import. This PR fixes a bug where this did not happen (instead the imported matrix's value is favored).
Azure · Mar 5, 2021 · c9b5add · c9b5add
1 parent d417685
commit c9b5add
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 11 deletions.
diff --git a/eng/common/scripts/job-matrix/job-matrix-functions.ps1 b/eng/common/scripts/job-matrix/job-matrix-functions.ps1
@@ -32,7 +32,7 @@ function GenerateMatrix(
     [Array]$filters = @(),
     [Array]$nonSparseParameters = @()
 ) {
-    $orderedMatrix, $importedMatrix = ProcessImport $config.orderedMatrix $selectFromMatrixType
+    $orderedMatrix, $importedMatrix, $importedDisplayNamesLookup = ProcessImport $config.orderedMatrix $selectFromMatrixType
     if ($selectFromMatrixType -eq "sparse") {
         [Array]$matrix = GenerateSparseMatrix $orderedMatrix $config.displayNamesLookup $nonSparseParameters
     } elseif ($selectFromMatrixType -eq "all") {
@@ -44,7 +44,7 @@ function GenerateMatrix(
     # Combine with imported after matrix generation, since a sparse selection should result in a full combination of the
     # top level and imported sparse matrices (as opposed to a sparse selection of both matrices).
     if ($importedMatrix) {
-        [Array]$matrix = CombineMatrices $matrix $importedMatrix
+        [Array]$matrix = CombineMatrices $matrix $importedMatrix $importedDisplayNamesLookup
     }
 
     if ($config.exclude) {
@@ -199,19 +199,19 @@ function ProcessIncludes([MatrixConfig]$config, [Array]$matrix)
 function ProcessImport([System.Collections.Specialized.OrderedDictionary]$matrix, [String]$selection)
 {
     if (!$matrix -or !$matrix.Contains($IMPORT_KEYWORD)) {
-        return $matrix
+        return $matrix, @(), @{}
     }
 
     $importPath = $matrix[$IMPORT_KEYWORD]
     $matrix.Remove($IMPORT_KEYWORD)
 
-    $matrixConfig = GetMatrixConfigFromJson (Get-Content $importPath)
-    $importedMatrix = GenerateMatrix $matrixConfig $selection
+    $importedMatrixConfig = GetMatrixConfigFromJson (Get-Content $importPath)
+    $importedMatrix = GenerateMatrix $importedMatrixConfig $selection
 
-    return $matrix, $importedMatrix
+    return $matrix, $importedMatrix, $importedMatrixConfig.displayNamesLookup
 }
 
-function CombineMatrices([Array]$matrix1, [Array]$matrix2)
+function CombineMatrices([Array]$matrix1, [Array]$matrix2, [Hashtable]$displayNamesLookup = @{})
 {
     $combined = @()
     if (!$matrix1) {
@@ -223,21 +223,22 @@ function CombineMatrices([Array]$matrix1, [Array]$matrix2)
 
     foreach ($entry1 in $matrix1) {
         foreach ($entry2 in $matrix2) {
+            $entry2name = @()
             $newEntry = @{
                 name = $entry1.name
                 parameters = CloneOrderedDictionary $entry1.parameters
             }
             foreach($param in $entry2.parameters.GetEnumerator()) {
-                if (!$newEntry.Contains($param.Name)) {
+                if (!$newEntry.parameters.Contains($param.Name)) {
                     $newEntry.parameters[$param.Name] = $param.Value
+                    $entry2name += CreateDisplayName $param.Value $displayNamesLookup
                 } else {
                     Write-Warning "Skipping duplicate parameter `"$($param.Name)`" when combining matrix."
                 }
             }
 
             # The maximum allowed matrix name length is 100 characters
-            $entry2.name = $entry2.name.TrimStart("job_")
-            $newEntry.name = $newEntry.name, $entry2.name -join "_"
+            $newEntry.name = @($newEntry.name, ($entry2name -join "_")) -join "_"
             if ($newEntry.name.Length -gt 100) {
                 $newEntry.name = $newEntry.name[0..99] -join ""
             }
@@ -305,7 +306,7 @@ function GenerateSparseMatrix(
 
     if ($nonSparse) {
         [Array]$allOfMatrix = GenerateFullMatrix $nonSparse $displayNamesLookup
-        return CombineMatrices $allOfMatrix $sparseMatrix
+        return CombineMatrices $allOfMatrix $sparseMatrix $displayNamesLookup
     }
 
     return $sparseMatrix

diff --git a/eng/common/scripts/job-matrix/tests/job-matrix-functions.modification.tests.ps1 b/eng/common/scripts/job-matrix/tests/job-matrix-functions.modification.tests.ps1
@@ -216,4 +216,25 @@ Describe "Platform Matrix Import" -Tag "import" {
         $matrix.Length | Should -Be 7
         CompareMatrices $matrix $expected
     }
+
+    It "Should generate a sparse matrix with an imported a sparse matrix" {
+        $matrixJson = @'
+{
+    "matrix": {
+        "$IMPORT": "./test-import-matrix.json",
+        "Foo": [ "fooOverride1", "fooOverride2" ],
+    }
+}
+'@
+
+        $importConfig = GetMatrixConfigFromJson $matrixJson
+        $matrix = GenerateMatrix $importConfig "sparse"
+
+        $matrix[0].parameters["Foo"] | Should -Be "fooOverride1"
+        $matrix[0].name | Should -Be "fooOverride1_bar1"
+        $matrix[3].parameters["Foo"] | Should -Be "fooOverride2"
+        $matrix[3].name | Should -Be "fooOverride2_bar1"
+        $matrix[5].parameters["Foo"] | Should -Be "fooOverride2"
+        $matrix[5].name | Should -Be "fooOverride2_importedBaz"
+    }
 }