Fix New-TestResources.ps1 to run pre/post deployment script #1876
Conversation
|
The following pipelines have been queued for testing: |
…albertcheng/new-testresources.ps1
This reverts commit d41510d.
|
The following pipelines have been queued for testing: |
|
Thanks for tracking this bug down! |
…m/ckairen/azure-sdk-tools into albertcheng/new-testresources.ps1
|
The following pipelines have been queued for testing: |
|
The following pipelines have been queued for testing: |
| @@ -167,11 +167,9 @@ try { | |||
| Get-ChildItem -Path $root -Filter "$_" -Recurse | ForEach-Object { | |||
| Write-Verbose "Found template '$($_.FullName)'" | |||
| if ($_.Extension -eq '.bicep') { | |||
| $templateFile = @{originalFilePath = $_.FullName; jsonFilePath = (BuildBicepFile $_)} | |||
| $templateFiles += $templateFile | |||
| $templateFiles += (BuildBicepFile $_) | |||
There was a problem hiding this comment.
I'm honestly not a fan of this change, @benbp. The temp file that gets returned and printed everywhere is an implementation detail of this script with no obvious tie-back to the original .bicep file. At the very least BuildBicepFile should print the source and destination paths, but that's still requiring users to look up through the logs to find it. How it was with the object I thought was a great idea, so you could even print both if someone needed to find the actual JSON template (I've ran into a few issues where this was needed with bicep files).
There was a problem hiding this comment.
I think you might be commenting on an outdated commit? Sorry for the messy commits.
There was a problem hiding this comment.
No, at the time I commented, the variable was just a single string path. You had it as a multi-property object but Ben asked you to change it. Did you chances it back to an object? We shouldn't be printing the path to a temp JSON file that the user can't use to diagnose failures.
There was a problem hiding this comment.
I think it is an outdated diff. The main code is object based: https://github.com/Azure/azure-sdk-tools/blob/main/eng/common/TestResources/New-TestResources.ps1#L170-L171
I think it may actually be helpful to print both paths, because when debugging bicep scripts, I often need to see the compiled output to help diagnose the problem.
There was a problem hiding this comment.
Yes, that's what I originally commented as well. The commit that had the original and transformed names as properties was best. My only comment from then was that the message printed when the two were different should list both. From the number of people I help set up their test resources (rather, customize them) it's clear expecting them to know how it works e.g., that it transforms the bicep file into json behind the scenes would be unnecessary.
|
Hello @azure-sdk! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
ghost
merged commit New-TestResources.ps1 was storing the .env file at the temp folder if the service uses bicep files. This bug fix stores the .env file at the correct service directory. (also pushed in logging request in #1876)
Fix bug in which New-TestResources.ps1 doesn't call test-resources-pre.ps1 and test-resources-post.ps1 script before and after template deployment from a bicep file.