馃尡 added tests for loadTLSConfigFromEnv in factory_test.go #1589
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
metal3-io-bot
added
the
needs-ok-to-test
|
Hi @danish9039. Thanks for your PR. I'm waiting for a metal3-io member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
metal3-io-bot
added
the
size/L
|
Please sign-off your commit to fulfill DCO requirement. /ok-to-test |
metal3-io-bot
added
ok-to-test
|
Thanks for the PR! |
metal3-io-bot
added
size/M
This commit introduces a set of comprehensive tests for the factory functions in the "factory_test.go" file of the ironic package. The tests cover various scenarios and edge cases related to loading configuration from environment variables, handling endpoints, and loading TLS configuration. Signed-off-by: danish siddiqui <danishsiddiqui040@gmail.com>
Signed-off-by: danish siddiqui <danishsiddiqui040@gmail.com>
Signed-off-by: danish siddiqui <danishsiddiqui040@gmail.com>
|
Could you update the description so it says "Part of #1574" instead of "Fixes", since the issue has multiple parts? |
sure |
| @@ -181,35 +181,78 @@ func TestLoadConfigFromEnv(t *testing.T) { | |||
| } | |||
| } | |||
|
|
|||
| func TestLoadEndpointsFromEnv(t *testing.T) { | |||
There was a problem hiding this comment.
Do I understand correctly that you removed this test? Why?
I think we should keep it and add a separate test for the TLS config.
| os.Setenv("IRONIC_CACERT_FILE", "/path/to/ca.crt") | ||
| os.Setenv("IRONIC_CLIENT_CERT_FILE", "/path/to/client.crt") | ||
| os.Setenv("IRONIC_CLIENT_PRIVATE_KEY_FILE", "/path/to/client.key") | ||
| os.Setenv("IRONIC_INSECURE", "false") | ||
| os.Setenv("IRONIC_SKIP_CLIENT_SAN_VERIFY", "true") | ||
| defer func() { | ||
| os.Unsetenv("IRONIC_CACERT_FILE") | ||
| os.Unsetenv("IRONIC_CLIENT_CERT_FILE") | ||
| os.Unsetenv("IRONIC_CLIENT_PRIVATE_KEY_FILE") | ||
| os.Unsetenv("IRONIC_INSECURE") | ||
| os.Unsetenv("IRONIC_SKIP_CLIENT_SAN_VERIFY") | ||
| }() |
There was a problem hiding this comment.
If you check the other tests in this file, you will see that they make use of a struct EnvFixture. There are helpful functions for setting and unsetting the environment from this struct. Put all of this together and you get a nice way of looping through test cases described by structs. Could you try to mimic this?
Signed-off-by: danish siddiqui <danishsiddiqui040@gmail.com>
This commit introduces a set of comprehensive tests for the in theloadingTLSconfiguration in"factory_test.go" file of the ironic package. The tests cover various scenarios and edge cases related to loading configuration from environment variables, handling endpoints, and loading TLSconfiguration.
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):part of #1574