Skip to content
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

fix: google_compute_global_forwarding_rule acc test #1514

Merged

Conversation

sundowndev-snyk
Copy link
Contributor

Q A
πŸ› Bug fix? no
πŸš€ New feature? no
⚠ Deprecations? no
❌ BC Break no
πŸ”— Related issues #...
❓ Documentation no

Description

This is an attempt to fix a failure in our acceptance test.

@sundowndev-snyk sundowndev-snyk added the kind/maintenance Refactoring or changes to the workspace label Jun 2, 2022
@sundowndev-snyk sundowndev-snyk requested a review from a team as a code owner June 2, 2022 07:34
@sundowndev-snyk sundowndev-snyk requested review from moadibfr and removed request for a team June 2, 2022 07:34
@codecov-commenter
Copy link

Codecov Report

Merging #1514 (741ed33) into main (0786e71) will increase coverage by 9.41%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1514      +/-   ##
==========================================
+ Coverage   72.37%   81.79%   +9.41%     
==========================================
  Files         514      439      -75     
  Lines       19338    16008    -3330     
==========================================
- Hits        13996    13093     -903     
+ Misses       5003     2598    -2405     
+ Partials      339      317      -22     
Impacted Files Coverage Ξ”
pkg/terraform/plugin_client.go 0.00% <0.00%> (-100.00%) ⬇️
pkg/remote/aws/client/s3_client_factory.go 0.00% <0.00%> (-100.00%) ⬇️
pkg/remote/aws/init.go 0.00% <0.00%> (-96.02%) ⬇️
pkg/resource/azurerm/azurerm_container_registry.go 12.50% <0.00%> (-87.50%) ⬇️
pkg/remote/azurerm/init.go 0.00% <0.00%> (-84.22%) ⬇️
pkg/remote/github/init.go 0.00% <0.00%> (-76.32%) ⬇️
pkg/iac/terraform/state/enumerator/azurerm.go 0.00% <0.00%> (-67.19%) ⬇️
pkg/terraform/provider_factory.go 0.00% <0.00%> (-62.50%) ⬇️
pkg/resource/aws/aws_route53_zone.go 46.15% <0.00%> (-53.85%) ⬇️
pkg/helpers/azure/storage.go 0.00% <0.00%> (-50.00%) ⬇️
... and 128 more

@@ -17,6 +18,8 @@ func TestAcc_Google_ComputeGlobalForwardingRule(t *testing.T) {
},
Checks: []acceptance.AccCheck{
{
// New resources are not visible immediately through GCP API after an apply operation.
ShouldRetry: acceptance.LinearBackoff(10 * time.Minute),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is that 10 minutes? quite long isn't it? from your experience it makes sense?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could probably switch it to 5min, but it's not that important because if in real life if it took less we'll get out of this without waiting 10 mins.
It's more like if we can do more we can do less in that case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

10 minutes is a reasonable time frame for an acceptance test IMO. Running the whole test suite takes more than 50 mins. Also like @eliecharra said, this is just a limit. In this case, 10 minutes belongs to 3 retries, while 5 minutes belongs to 2 retries.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool sounds great πŸ‘

@sundowndev-snyk sundowndev-snyk merged commit dd2f210 into main Jun 7, 2022
@sundowndev-snyk sundowndev-snyk deleted the fix/google_compute_global_forwarding_rule_acc_test branch June 7, 2022 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/maintenance Refactoring or changes to the workspace
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants