Make pytest.mark.parametrize more readable in this particular case

[zhukovgreen]

Current situation:

   # valid cases without RHSM (== skip_rhsm)                                   
    (None, '1', testInData(_PACKAGES_MSGS, None, _XFS_MSG, _STORAGEINFO_MSG, None)),
    (None, '1', testInData(_PACKAGES_MSGS, None, None, _STORAGEINFO_MSG, None)),
    (None, '1', testInData([], None, _XFS_MSG, _STORAGEINFO_MSG, None)),        
    (None, '1', testInData([], None, None, _STORAGEINFO_MSG, None)),            
    (None, '1', testInData(_PACKAGES_MSGS, None, _XFS_MSG, _STORAGEINFO_MSG, _CTRF_MSGS)),
    (None, '1', testInData(_PACKAGES_MSGS, None, None, _STORAGEINFO_MSG, _CTRF_MSGS)),
    (None, '1', testInData([], None, _XFS_MSG, _STORAGEINFO_MSG, _CTRF_MSGS)),  
    (None, '1', testInData([], None, None, _STORAGEINFO_MSG, _CTRF_MSGS)),  

Example of more readable version:

@pytest.mark.parametrize('raised,no_rhsm,testdata', [                           
    # valid cases with RHSM                                                     
    (None, '0', testInData(_PACKAGES_MSGS   , _RHSMINFO_MSG, _XFS_MSG,  _STORAGEINFO_MSG, None)),
    (None, '0', testInData(_PACKAGES_MSGS[0], _RHSMINFO_MSG, _XFS_MSG,  _STORAGEINFO_MSG, None)),
    (None, '0', testInData([],                _RHSMINFO_MSG, _XFS_MSG,  _STORAGEINFO_MSG, None)),
    (None, '0', testInData(_PACKAGES_MSGS,    _RHSMINFO_MSG, None,      _STORAGEINFO_MSG, None)),
    (None, '0', testInData(_PACKAGES_MSGS,    _RHSMINFO_MSG, _XFS_MSG,  _STORAGEINFO_MSG, _CTRF_MSGS)),
    (None, '0', testInData(_PACKAGES_MSGS[0], _RHSMINFO_MSG, _XFS_MSG,  _STORAGEINFO_MSG, _CTRF_MSGS)),
    (None, '0', testInData([],                _RHSMINFO_MSG, _XFS_MSG,  _STORAGEINFO_MSG, _CTRF_MSGS)),
    (None, '0', testInData(_PACKAGES_MSGS,    _RHSMINFO_MSG, None,      _STORAGEINFO_MSG, _CTRF_MSGS)),

Originally posted by @pirat89 in https://github.com/oamg/leapp-repository/pull/539

[zhukovgreen]

@pirat89 I need a bit more context here to understand the problem. Could you please check the name of the ticket and refactor its description? Or it is that simple as the name?:)

[pirat89]

@ZhukovGreen I thought you reacted to the testing of of consumed data (the test_consume_data test) in the targetuserspacecreator that is really not readable so much :) There is nothing about "test data is read only". It was just about readability of the unit-test ;-) I have been thinking about that during the last night and I think we should use the table like formatting for every case, ignoring multiple whitespaces, more than 120chars par line, etc. linters stuff, like:

@pytest.mark.parametrize('raised,no_rhsm,testdata', [                           
    # valid cases with RHSM                                                     
    (None, '0', testInData(_PACKAGES_MSGS   , _RHSMINFO_MSG, _XFS_MSG,  _STORAGEINFO_MSG, None)),
    (None, '0', testInData(_PACKAGES_MSGS[0], _RHSMINFO_MSG, _XFS_MSG,  _STORAGEINFO_MSG, None)),
    (None, '0', testInData([],                _RHSMINFO_MSG, _XFS_MSG,  _STORAGEINFO_MSG, None)),
    (None, '0', testInData(_PACKAGES_MSGS,    _RHSMINFO_MSG, None,      _STORAGEINFO_MSG, None)),
    (None, '0', testInData(_PACKAGES_MSGS,    _RHSMINFO_MSG, _XFS_MSG,  _STORAGEINFO_MSG, _CTRF_MSGS)),
    (None, '0', testInData(_PACKAGES_MSGS[0], _RHSMINFO_MSG, _XFS_MSG,  _STORAGEINFO_MSG, _CTRF_MSGS)),
    (None, '0', testInData([],                _RHSMINFO_MSG, _XFS_MSG,  _STORAGEINFO_MSG, _CTRF_MSGS)),
    (None, '0', testInData(_PACKAGES_MSGS,    _RHSMINFO_MSG, None,      _STORAGEINFO_MSG, _CTRF_MSGS)),

Just this change makes all test cases much readable and easier to manage. WDYT?

For comparison of readability (another block without table-like formatting:

   # valid cases without RHSM (== skip_rhsm)                                   
    (None, '1', testInData(_PACKAGES_MSGS, None, _XFS_MSG, _STORAGEINFO_MSG, None)),
    (None, '1', testInData(_PACKAGES_MSGS, None, None, _STORAGEINFO_MSG, None)),
    (None, '1', testInData([], None, _XFS_MSG, _STORAGEINFO_MSG, None)),        
    (None, '1', testInData([], None, None, _STORAGEINFO_MSG, None)),            
    (None, '1', testInData(_PACKAGES_MSGS, None, _XFS_MSG, _STORAGEINFO_MSG, _CTRF_MSGS)),
    (None, '1', testInData(_PACKAGES_MSGS, None, None, _STORAGEINFO_MSG, _CTRF_MSGS)),
    (None, '1', testInData([], None, _XFS_MSG, _STORAGEINFO_MSG, _CTRF_MSGS)),  
    (None, '1', testInData([], None, None, _STORAGEINFO_MSG, _CTRF_MSGS)),  

[pirat89]

Just to see how it could looks like, I created draft of the PR - still problems with flake8, but just for the purpose of discussion good enough. no need to spend too much time on this right now.

[zhukovgreen]

@pirat89 thx for this demo MR. I got the problem. I know two more ways to pass the testing grid into parametrize. I will look into it tommorrow

[zhukovgreen]

@pirat89 I was unable to find anything better than just adopting a list of dicts as a testing case map. Something like this would work:

import pytest

TEST_MAP = [
    dict(a=1, b=2, exp=3),
    dict(a=2, b=3, exp=5),
    dict(a=2, b=2, exp=4),
]

@pytest.mark.parametrize(
    tuple(TEST_MAP[0]), ((test.values()) for test in TEST_MAP)
)
def test_parametrization(a, b, exp):
    assert a + b == exp

However, I don't really see it worse the effort:)

Another approach is so complex than I decided to skip it from the investigation. But just for the sake of awareness, this is using metafunc.parametrize inside the pytest_test_generate hook. In this case, you can adopt any datastructure to make it work with the pytest. But again, in python such formatting like you did in #557 is pretty rare I guess

[zhukovgreen]

Voting to close the ticket, wdyt @pirat89 ?

[pirat89]

I have no problem with closing, but we can keep it opened for now and spare a discussion around that when we will have more time. But it's purely nice to have.

[zhukovgreen]

fine to me @pirat89

[pirat89]

As there was not progress in the last year, let's close this. Maybe the matrix will be fixed automatically when the actor is refactored.