AzureRT - Add Dynamic Tests for Project and/or Test Runner #448
Conversation
|
Hi @huangpf, I'm your friendly neighborhood Azure Pull Request Bot (You can call me AZPRBOT). Thanks for your contribution!
TTYL, AZPRBOT; |
huangpf
changed the title
huangpf
changed the title
huangpf
changed the title
| public partial class VMDynamicTests | ||
| { | ||
| [Fact] | ||
| [Trait(Category.AcceptanceType, Category.CheckIn)] |
There was a problem hiding this comment.
Can we add another AcceptanceType for dynamic tests to distinguish runner tests from other tests?
There was a problem hiding this comment.
This test actually can be a check-in test in Playback mode.
There was a problem hiding this comment.
When the runner gets this test script and run it on the runner machine, it will be in Live mode.
|
retest this please |
|
|
||
| for ($i = 0; $i -lt $locations.Count; $i++) | ||
| { | ||
| $locations[$i] = ($locations[$i] -Replace ' ', '').ToLower(); |
There was a problem hiding this comment.
Why do you remove space and change it to lower case? Should the location name be used as it is?
There was a problem hiding this comment.
Yes, the canonical form (instructed by the ARM team) is to remove any white spaces and use lower cases.
There was a problem hiding this comment.
Is it a bug in ARM? What happened if we don't remove space? I think creating a VM with 'West US' should work.
There was a problem hiding this comment.
I'm not sure whether it's a bug, and it's just safe to use this form. Creating VM is fine, but Get-AzureVMSize -Location "West US" would fail.
There was a problem hiding this comment.
I think you already fixed Get-AzureVMSize such that it works with "West US". Is there any other cmdlet that does not change to a canonical form and fail with "West US" ?
There was a problem hiding this comment.
Good point. But there may be other cmdlets that are not compatible yet.
May I know what's your concern if only the canonical form is used?
There was a problem hiding this comment.
If a cmdlet cannot handle a standard location name, I consider it a bug. A customer should not be worried about canonical form and must be able to use a location name from Get-AzureLocation. (Is there any document that a customer should convert a location name to a canonical form for a certain case?) So, any test should not convert a standard location name into a canonical name. It should be handled by cmdlet (or by client library or by server.)
There was a problem hiding this comment.
Well, I don't think all users must use the return from Get-AzureLocation, and for example, I may simply know that westus works, but I can understand your suggestion -- you would like to validate that the return values from Get-AzureLocation can be used here.
I would argue that the dynamic tests' focus is not yet about different forms of location strings (for which we have different test cases), as VM create/remove scenario is.
I would suggest we create a new item for follow-up.
There was a problem hiding this comment.
Ok, I just wonder if this dynamic test fails if we use only standard location name instead of canonical name. (If fails, where?)
There was a problem hiding this comment.
I can try and let you know, :-)
If there is no error, I would update the PR, ;-)
AzureRT - Add Dynamic Tests for Project and/or Test Runner
HPF PR: dev <- Azure:dev
No description provided.