Skip to content

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

@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