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

[WIP] Add initial Redfish unit tests #398

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

matejart
Copy link
Contributor

@matejart matejart commented Jul 15, 2019

This PR adds the unit tests for the Redfish systems API. This will work with python 3 only.
I added a step in the travis configuration that detects the current Python version and disables the tests if needed.

The API for Redfish is now extended to also include the new physical server properties introduced to the GUI in CloudForms 5.0.

@mshriver mshriver self-assigned this Jul 15, 2019
@matejart matejart force-pushed the redfish-unittests branch 2 times, most recently from 9a6e2a1 to 3ced153 Compare July 16, 2019 09:54
@matejart
Copy link
Contributor Author

@mshriver Travis is now passing in Python 3. Do you have any advice how to disable the tests that only work in Python 3 for the Python 2 version of tests?

Added unit tests for Redfish system. These unit tests employ
`unittest.mock`, which is a Python 3 feature only.
This adds `machine_type` and `product_name` properties to
`RedfishServer`.

Related PR:
  * ManageIQ/manageiq-providers-redfish#83
We address Python 3's `Exception` not having `message` anymore.
@matejart matejart force-pushed the redfish-unittests branch 4 times, most recently from 55025d1 to 0f23bcc Compare August 28, 2019 11:08
The Redfish tests are Python 3 only. With this change we remove
the tests if running in anything less than Python 3.6.
@matejart
Copy link
Contributor Author

The Travis checks are now passing. In the Python 3.7 configuration, it runs the new unit tests.

@miha-plesko please have a look.

@dajoRH dajoRH added needs-lint and removed lint-ok labels Aug 29, 2019
@miha-plesko
Copy link
Contributor

@matejart I'm overwhelmed with other projects, especially now that I've returned from a whole week of sick leave, so I'm passing the pleasure of the code review straight to the @mshriver .

On a quick scroll, however, I think you should really use pytest marker to conditionally skip tests when python version is not high enough, e.g. http://doc.pytest.org/en/latest/skipping.html#id1

@pytest.mark.skipif(sys.version_info < (3, 6), reason="requires python3.6 or higher")
def test_function():
    ...

@matejart
Copy link
Contributor Author

matejart commented Sep 2, 2019

Let's still review it iternally first.

I tried using @pytest.mark, but that doesn't work because pytest fails on from unittest import mock before it even gets to the (un)collection. Since unittest.mock is a Python 3 thing, and with the overall migration to Python 3.7, I was quicker and cleaner with the rm script.

This commit replaces the use of unittest with the constructs from
pytest. We started off using unittest, but then we'd introduce
parametrization, which wouldn't work because the mix with unit test
caused parametrization to get lost. Now we use mainly the equivalent
pytest constructs.
@mshriver
Copy link
Collaborator

mshriver commented Sep 6, 2019

@matejart @miha-plesko let's make this even easier. I'm going to drop support for python <3.7 here, as I believe all teams using this library have migrated to py3 by now

Check out #412

@ogajduse ogajduse marked this pull request as draft October 11, 2023 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants