-
Notifications
You must be signed in to change notification settings - Fork 47
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
feat: Adding Port2IBM2.0 example and test scripts #426
Conversation
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #426 +/- ##
==========================================
+ Coverage 60.11% 60.15% +0.03%
==========================================
Files 99 99
Lines 20045 20050 +5
==========================================
+ Hits 12051 12061 +10
+ Misses 7691 7687 -4
+ Partials 303 302 -1
☔ View full report in Codecov by Sentry. |
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'm curious if naming the directory for the example ibm2.0 is going to cause issues somewhere; potentially in the modules. I don't think all tools will have an issue with it but I also don't know which tools might. Other than that there's just one small change requested and the rest looked good.
tests/connection_e2e_ibm_test.go
Outdated
@@ -10,7 +10,7 @@ import ( | |||
func TestIBMCreateConnection(t *testing.T) { | |||
// retryable errors in terraform testing. | |||
terraformOptions := terraform.WithDefaultRetryableErrors(t, &terraform.Options{ | |||
TerraformDir: "../examples/fabric/v4/portConnectivity/ibm", | |||
TerraformDir: "../examples/fabric/v4/portConnectivity/ibm/ibm2.0", |
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 should update the filename to ibm2 as well because we'll add an E2E test for ibm1 after it's added.
} else { | ||
updatedConn = conn | ||
} |
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.
doing it this way, in case of error it will not save the last status
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, this is intentional; some of the updates might fail but we want to save the status of the last successful update to the connection. We're not worried about the status of the failed update. The response won't have any details we're concerned about saving to Terraform state.
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.
LGTM!
Port 2 IBM2.0 Connection example Scripts
Added test script for FCR Multi Cloud Connection