-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Add cross product matrix tooling #17417
Conversation
Copying this PR over from Azure/azure-sdk-tools#1254, since we want to land it in net before the shared tools repo (easier iteration). @weshaggard Most of your comments from there have been addressed. Two updates:
|
463e3d1
to
cd5d9c9
Compare
@@ -1,8 +1,7 @@ | |||
Using Module ./test-matrix-functions.psm1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to use a PS module here? In general we have moved away from using PS modules for our helper scripts as they tend to be more trouble then they are worth. If there isn't a clear reason I'd suggest just dot-sourcing the file instead of importing it as a module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import-Module and dot sourcing don't import class definitions, which I use in the tests for type assertions and casting of test objects. I could write the code without these classes (one of which is used for config validation, and the other as a type shortcut) but there's so much various object marshalling here that it's getting unwieldy to maintain without introducing types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I misread that I assumed this was Import-Module. I don't think I've used the "using module" feature before, not exactly sure what it does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it have to be a module to support the class import? If not I still think it might be better to use ps1 instead of psm1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it seems to fail when run against a ps1
, hence the rename:
$ ls
test-matrix-functions.ps1 test-matrix-functions.psm1
$ using module ./test-matrix-functions.ps1
InvalidOperation: /home/ben/sdk/azure-sdk-for-net/eng/scripts/job-matrix/test/test-matrix-functions.ps1:3
Line |
3 | class OrderedDictionary : System.Collections.Specialized.OrderedDicti …
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| Unable to find type [OrderedDictionary].
$ using module ./test-matrix-functions.psm1
$ get-module
ModuleType Version PreRelease Name ExportedCommands
---------- ------- ---------- ---- ----------------
Manifest 7.0.0.0 Microsoft.PowerShell.Management {Add-Content, Clear-Content, Clear-Item, Clear-ItemProperty…}
Manifest 7.0.0.0 Microsoft.PowerShell.Utility {Add-Member, Add-Type, Clear-Variable, Compare-Object…}
Script 2.0.2 PSReadLine {Get-PSReadLineKeyHandler, Get-PSReadLineOption, Remove-PSReadLineKeyHandler, Set-PSReadLineKeyHandler…}
Script 0.0 test-matrix-functions
Script 0.0 test-matrix-functions {ConvertToMatrixArrayFormat, CreateDisplayName, CreateMatrixEntry, GenerateFullMatrix…}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think using
is new as of powershell 5.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could also split the class definitions out into their own separate module file, and keep all the functions dot-sourcable.
/check-enforcer evaluate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking good and ready to for some testing. I've scanned the code it is hard to wrap my head around all of it without planning with it. So looks good beyond the module comment. Time to get testing.
* Initial commit of test matrix generator * Add multiple matrix job template * Use powershell task syntax for matrix template * Use inline syntax for matrix powershell functions * Rename test-matrix samples and scripts to job-matrix * [job matrix] Enforce naming restrictions. Extend matrix template in sample * Switch matrix config processing to follow json declaration order * Remove locale from job-matrix README links * Fixes for samples and template
This adds scripts, docs and samples supporting dynamic, cross-product matrix generation for azure pipeline jobs.
It aims to replicate the cross-product matrix functionality in github actions,
but also adds some additional features like sparse matrix generation, cross-product includes and excludes, and (TODO) matrix filters.
This functionality is made possible by the ability for the azure pipelines yaml to take a dynamic variable as an input
for a job matrix definition (see the code sample at the bottom of the linked section).
See the README.md file for more details on the config file syntax and usage, as well as implementation details.
The tests (
test-matrix-functions.tests.ps1
) contain a lot of detail on expected data structures at various processing stages. The-
test-matrix-functions.ps1` file could perhaps be split up or use some more organization, so let me know if it's hard to navigate.Example:
Related discussion: microsoft/azure-pipelines-yaml#20