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.

Sign up for GitHub

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

Jump to bottom

refactor: Use parameterized in redfish unit tests. #55

Merged
merged 2 commits into from
Dec 6, 2023
Merged

refactor: Use parameterized in redfish unit tests. #55

merged 2 commits into from
Dec 6, 2023
+39 −120

Conversation

dashmage
Copy link
Contributor

@dashmage dashmage commented Dec 4, 2023

Use the parameterized package in the test_redfish.py unit tests. This allows us to more easily create similar looking tests which only differ in the parameters being passed to them.

Resolves #53

@dashmage
Parameterize storage name tests.
9524815 
This includes `get_storage_controller_data_success` and
`get_storage_drive_data_success`.

Also removes the redundant `test_non_standard_storage_uri_name`.
@dashmage
Parameterize other unit tests.
5e42442 
* `test_redfish_helper_context_manager_fail`
* `test_redfish_available_login_fail`
@dashmage dashmage requested review from a team, Pjack, aieri, agileshaw, jneo8 and rgildein December 4, 2023 17:21
Pjack
Pjack requested changes Dec 5, 2023
View reviewed changes
Copy link

@Pjack Pjack left a comment

Choose a reason for hiding this comment

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

Thanks! It's a good refactor. And after the refactor, I also realize we miss some test case for the fail case. Could you help to add them? thanks!
get_storage_drive_data_fail
get_storage_controller_data_fail

@Pjack Pjack mentioned this pull request Dec 5, 2023
Merged
@dashmage
Copy link
Contributor Author

dashmage commented Dec 5, 2023 •
edited
Loading

Thanks! It's a good refactor. And after the refactor, I also realize we miss some test case for the fail case. Could you help to add them? thanks! get_storage_drive_data_fail get_storage_controller_data_fail

Sure thing, good suggestion. Although in this PR, I'd like to contain the scope and not add any new tests, just refactor existing ones to make it more cleaner using the parameterized package.

I'll create a new issue and will add the tests as part of that one :)

Update: Created #56

rgildein
rgildein approved these changes Dec 5, 2023
View reviewed changes
Copy link
Contributor

@rgildein rgildein left a comment

Choose a reason for hiding this comment

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

I like the usage of parametrize, and this changes are LGTM to me.

tests/unit/test_redfish.py Show resolved Hide resolved
@dashmage dashmage merged commit e730e7d into canonical:main Dec 6, 2023
3 checks passed
@dashmage dashmage deleted the refactor-53/use-parameterized branch December 6, 2023 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rgildein rgildein rgildein approved these changes

@Pjack Pjack Pjack approved these changes

@aieri aieri Awaiting requested review from aieri

@agileshaw agileshaw Awaiting requested review from agileshaw

@jneo8 jneo8 Awaiting requested review from jneo8

Assignees

@dashmage dashmage

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Refactor tests with parameterized
3 participants
@dashmage @Pjack @rgildein