Remove hardcoded values that are handled by the API #11423
Conversation
|
Hello! I am a robot. Tests will require approval from a repository maintainer to run. @rileykarson, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look. You can help make sure that review is quick by doing a self-review and by running impacted tests locally. |
modular-magician
added
awaiting-approval
Tests analyticsTotal tests: 979 Click here to see the affected service packages
Action takenFound 170 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
|
|
|
This was messing up the order of the disks in terraform state else if v := disk["type"]; v.(string) == "SCRATCH" {
iface := disk["interface"].(string)
if iface == "" {
// apply-time default
iface = "SCSI"
}
scratchDisksByInterface[iface] = append(scratchDisksByInterface[iface], i)
}Tested the default values and the config from this test after the fix with |
modular-magician
added
awaiting-approval
Tests analyticsTotal tests: 980 Click here to see the affected service packages
Action takenFound 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
|
|
|
@rileykarson This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
|
The unit test was working based on the hardcoded values that are currently removed |
modular-magician
added
the
awaiting-approval
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 1 week. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 2 weeks. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 3 weeks. Please take a look! Use the label |
|
Apologies, I've seen this and have run the tests a couple times but haven't had the cycles to sit down and exhaustively determine if it's safe or not. This is on my radar. |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 4 weeks. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 5 weeks. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 6 weeks. Please take a look! Use the label |
|
@karolgorc please see this comment: #11742 (comment) If you would like assistance converting files, we need your permission to push changes to this branch. |
fix unit test expecting hardcoded values fix ordering scratch disks in the state Remove hardcoded values that are handled by the API
c2thorn
force-pushed
the
remove-hardcoded-values-instance-template
branch
from
d2f04f4 to
f6824eb
Compare
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 7 weeks. Please take a look! Use the label |
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Tests analyticsTotal tests: 1017 Click here to see the affected service packages
Action takenFound 173 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
|
|
|
rerunning |
|
/gcbrun |
modular-magician
added
the
awaiting-approval
|
all the changes from the backup seem present: https://github.com/c2thorn/magic-modules/commits/remove-hardcoded-values-instance-template-11423-backup/ |
|
These test errors are probably some conflicts due to the fact that this PR was stale for so much time. Will look into these |
|
Regarding the failing tests, from another PR |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 8 weeks. Please take a look! Use the label |
|
Forgot about the region_instance_template with the unit-test changes. Will update
|
|
Maybe a custom check that compares the disk configuration between 2 templates. And then testcases creating templates:
Ideally there should be no differences because the hardcoded values removed are the same defaults as in the API. |
|
@rileykarson This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 1 week. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 2 weeks. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 3 weeks. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 4 weeks. Please take a look! Use the label |
|
Hi can this get reviewed or reassigned? |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 5 weeks. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 6 weeks. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 7 weeks. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 8 weeks. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 9 weeks. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 10 weeks. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 11 weeks. Please take a look! Use the label |
Resolves Issue #5295
buildDisks()for instance_templatesSetting this to defaults is handled by the instance.Insert() API method already
Release Note Template for Downstream PRs (will be copied)