CiscoDevNet / terraform-provider-aci

Terraform Cisco ACI provider
https://registry.terraform.io/providers/CiscoDevNet/aci/latest/docs
Mozilla Public License 2.0
90 stars 102 forks source link

Addition of new generated resource and data source for pkiTP (DCNE-143 DCNE-142) #1145

Closed shrsr closed 1 month ago

codecov-commenter commented 5 months ago

Codecov Report

Attention: Patch coverage is 88.63854% with 237 lines in your changes missing coverage. Please review.

Project coverage is 85.06%. Comparing base (3722887) to head (cdbb174). Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
internal/provider/resource_aci_key_ring.go 85.13% 62 Missing and 34 partials :warning:
...nal/provider/resource_aci_certificate_authority.go 82.98% 60 Missing and 30 partials :warning:
internal/provider/function_compare_versions.go 72.32% 21 Missing and 10 partials :warning:
.../provider/data_source_aci_certificate_authority.go 92.98% 6 Missing and 2 partials :warning:
internal/provider/data_source_aci_key_ring.go 94.36% 6 Missing and 2 partials :warning:
internal/provider/resource_aci_application_epg.go 96.55% 2 Missing :warning:
...l/provider/resource_aci_endpoint_security_group.go 96.87% 1 Missing :warning:
...al/provider/resource_aci_netflow_monitor_policy.go 94.73% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1145 +/- ## ========================================== + Coverage 85.05% 85.06% +0.01% ========================================== Files 102 107 +5 Lines 37889 39446 +1557 ========================================== + Hits 32226 33556 +1330 - Misses 4192 4340 +148 - Partials 1471 1550 +79 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

shrsr commented 5 months ago

@samiib Thanks for running the CI test for this PR. It's a big relief that the test is passing on v6.0

Yeah, I am aware of this issue. When I introduced the test_type functionality we still didn't have a CI in place for TF resource testing that could run tests on multiple versions of APIC.

I agree. At this juncture the easy and practical solution is to skip the entire test for a resource on an older version of APIC where the test contains at least one property that is only supported on the latest version of APIC. We need to make a few changes in the generator and the test template for this.

Yes, we have a cloud APIC but I'm not sure if we should test resources against the cAPIC since it has reached its EoL.

akinross commented 5 months ago

please also rebase once https://github.com/CiscoDevNet/terraform-provider-aci/pull/1236 is merged

akinross commented 3 months ago

needs rebase