search

neoave / mrack

Multicloud use-case based multihost async provisioner for CIs and testing during development
Apache License 2.0
11 stars 14 forks source link

fix: Owner requirement boolean parsing from string #223

Closed Tiboris closed 1 year ago

Tiboris commented 1 year ago

When parsing boolean values from string using bool() builtin funtion string which is not empty returns True Writing a util function to convert such strings to proper boolean values. NOTE: distutil implementation: from distutils.util import strtobool will be deprecated and removed in future python releases thus implementing a funtion ourselves might be future proof solution at this time.

Signed-off-by: Tibor Dudlák tdudlak@redhat.com

pvoborni commented 1 year ago

value_to_bool looks like, that it is a perfect candidate for a unit test ;)

Tiboris commented 1 year ago

I thought that when affected mrack util unit tests were not wrapped by Class It caused that they were not run. The old run of pytest:

https://dev.azure.com/neoave/mrack/_build/results?buildId=816&view=logs&j=27008f10-b491-547d-a654-20cfee07d03e&t=73113a39-43fd-502e-b4fb-b68667aceda6&l=330

======================= 81 passed, 22 warnings in 1.32s ========================

Adding the class and waiting for new run.

Locally with the patch i see test run (upstream missing)

tests/unit/test_utils.py ...............................

@miskopo Could you try to help me find what I am missing?

miskopo commented 1 year ago

I thought that when affected mrack util unit tests were not wrapped by Class It caused that they were not run. The old run of pytest:

https://dev.azure.com/neoave/mrack/_build/results?buildId=816&view=logs&j=27008f10-b491-547d-a654-20cfee07d03e&t=73113a39-43fd-502e-b4fb-b68667aceda6&l=330

======================= 81 passed, 22 warnings in 1.32s ========================

Adding the class and waiting for new run.

Locally with the patch i see test run (upstream missing)

tests/unit/test_utils.py ...............................

@miskopo Could you try to help me find what I am missing?

So, I have checked the logs and it seems like everything is in order except for the fact, that the output is truncated. If you look at test log, the last test with full output is at 61% and then it stops, however, test_pytest_multihost.py only has one test case, so it doesn't make sense to be the last.

I have also tested loccaly, both with tox -e py and pytest -v -r to ensure it's not just some config error and I get consistently 81 tests being run.

To conclude, I think the test is actually being run. To test this hypothesis, you can mark one as fail and you should see it in the failure log.

kaleemsiddiqu commented 1 year ago

Though not related with this but we should one test for osp env as well like for get_username(host_win_aws, metahost_win, provisioning_config) Rest LGTM

Tiboris commented 1 year ago

Thanks for review !