-
Couldn't load subscription status.
- Fork 4
Add SelectByName parameter #148
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
base: master
Are you sure you want to change the base?
Conversation
tasks/run-matlab-tests/v1/task.json
Outdated
| "defaultValue": "", | ||
| "groupName": "filterTests", | ||
| "required": false, | ||
| "helpMarkDown": "Test name used to select test suite elements. To create a test suite, the task uses only the test elements with the specified name." |
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.
Thanks for adding this text, Vahila.
@davidbuzinski: Where would such text show up? In the UI?
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.
It shows up if you try to edit the yaml in the pipeline editor at dev.azure.com. There is a list of all the input fields, and each field has a info icon that you can hover over to see this help text.
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.
Ok, thanks David.
integ-test-promote-template.yml
Outdated
| set -e | ||
| grep -q StartupTest test-results/matlab/selectbyname.xml | ||
| grep -q simpleTest test-results/matlab/selectbyname.xml | ||
| ! grep -q FirstTest test-results/matlab/selectbytag.xml |
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.
Should this be ! grep -q FirstTest test-results/matlab/selectbytag.xml?
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.
Did you mean to remove the !?
This is to verify that the 'FirstTest' test is not run so used a ! before the grep command.
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 forget what I meant, but I think I meant to type selectbyname.xml. Should they all be checking in one .xml for the select by name run?
tasks/run-matlab-tests/v1/task.json
Outdated
| "defaultValue": "", | ||
| "groupName": "filterTests", | ||
| "required": false, | ||
| "helpMarkDown": "Test name used to select test suite elements. To create a test suite, the task uses only the test elements with the specified name." |
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.
It shows up if you try to edit the yaml in the pipeline editor at dev.azure.com. There is a list of all the input fields, and each field has a info icon that you can hover over to see this help text.
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.
Fix it, then ship it! We should really fix scriptgen though so that we do not have to do the error prone space-separated list to cell array conversion in each integration.
| } | ||
|
|
||
| // Function to convert space separated names to cell array of character vectors | ||
| export function getSelectByNameAsCellArray(input?: string): string { |
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.
Hopefully this is just temporary until scriptgen gets updated to properly accept SelectByName as a space separated string?
While unlikely, single-quotes in the strings need to be escaped or it may result in a malformed cell array.
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.
Yes, I created an issue in scritgen repo and added a note to update this once that issue is resolved.
| `'CoberturaModelCoverage','${options.CoberturaModelCoverage || ""}',` + | ||
| `'SelectByTag','${options.SelectByTag || ""}',` + | ||
| `'SelectByFolder','${options.SelectByFolder || ""}',` + | ||
| `'SelectByName',${getSelectByNameAsCellArray(options.SelectByName)},` + |
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 plan to go back and fix scriptgen so this can be:
'SelectByName','${options.SelectByName || ""}', ?
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.
Yes, created an issue under scriptgen repo.
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.
Still have that question about the one step in the pipeline yml, but if that's correct, then go ahead and ship it!
tasks/run-matlab-tests/v1/task.json
Outdated
| "defaultValue": "", | ||
| "groupName": "filterTests", | ||
| "required": false, | ||
| "helpMarkDown": "Test name used to select test suite elements. To create a test suite, the task uses only the test elements with the specified name." |
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.
@Vahila : Would you like to replaced "with the specified name" with "that match the specified name"?
Added new 'SelectByName' parameter. This would help in supporting Parallel Strategy in Azure.
Note: Genscript changes to support SelectByName needs to be available before these can be merged.