Unit test coverage improvement in BMO

[lentzi90]

Improve the unit test coverage by writing new test cases or completely new tests. Check the current coverage with make unit-cover.

Here are some suggestions on where to start. Note that some functions are harder to test than others because they may depend on access to network resources or a database. If you come across such function, skip them and aim for the low hanging fruits instead.

• features.go has no tests at all and looks quite simple to test. Fixed in 🌱 Add unit test on features.go #1579
• loadTLSConfigFromEnv is untested and should be easy to test. Fixed in WIP: 🌱 added tests for loadTLSConfigFromEnv in factory_test.go #1589 or 🌱 Improve test coverage #2013
• baremetalhost_types.go has multiple small functions without coverage that could easily be tested. Fixed in 🌱Added unit tests for baremetalHostTypes and bmceventSubscriptionController #1615
• bmceventsubscription_controller.go has no tests at all. Fixed in 🌱Added unit tests for baremetalHostTypes and bmceventSubscriptionController #1615
• preprovisioningimage_controller.go has no tests at all. Fixed in 🌱 Improve test coverage #2013
• LoadAuth is untested. Fixed in 🌱 Add unit test for LoadAuth funciton in auth.go #1774
• client.go has no tests at all. Fixed in 🌱 Add unit test on pkg/provisioner/ironic/client.go #1606

[lentzi90]

/triage accepted
/good-first-issue

[metal3-io-bot]

@lentzi90:
This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

In response to this:

/triage accepted
/good-first-issue

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.

[lentzi90]

/reopen

[metal3-io-bot]

@lentzi90: Reopened this issue.

In response to this:

/reopen

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.

[NeerajNagure]

/assign NeerajNagure

[lentzi90]

/reopen
There are a few items in the list still

[metal3-io-bot]

@lentzi90: Reopened this issue.

In response to this:

/reopen
There are a few items in the list still

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.

[babugeet]

@lentzi90 : can i take it up for adding test cases for LoadAuth

[lentzi90]

Sure! Go ahead @babugeet ! 🙂

[babugeet]

/assign babugeet
@lentzi90 : pls review the above PR

The provided test cases cover the following scenarios for the LoadAuth function:

No Authentication Directory Exists:
    Test Case: NoAuthDirectory
    Description: Tests the scenario where the ironic authentication directory does not exist. The function should return NoAuth as the authentication type without any errors.

Valid Authentication Credentials:
    Test Case: ValidAuth
    Description: Tests the scenario where the ironic directory exists with valid username and password files. The function should return HTTPBasicAuth with the correct username and password.

Empty Username File:
    Test Case: EmptyUsername
    Description: Tests the scenario where the username file is present but empty. The function should return an error indicating that the HTTP Basic Auth username is empty.

Empty Password File:
    Test Case: EmptyPassword
    Description: Tests the scenario where the password file is present but empty. The function should return an error indicating that the HTTP Basic Auth password is empty.

[metal3-io-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues will close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

/lifecycle stale

[Rozzii]

/remove-lifecycle stale

[Rozzii]

@lentzi90 this is still open, can we close this ? If not I would like to move this to the general roadmap.

[lentzi90]

Yes let's keep it open until we finish the last few points on the list