Add exclude parameter

[grisu48]

Adds a new --exclude parameter to img-proof test, that allows to pass a comma-separated list of test expressions to skip during a test run.

This allows for using of common patterns e.g. test_sles without running certain tests. Before one needed to state an explicit list of test runs with the risk of not including new test runs as they are included into img-proof.

[grisu48]

pytest --cov=img_proof passes and the coverage is above 90%:

---------- coverage: platform linux, python 3.10.5-final-0 -----------
Name                             Stmts   Miss  Cover
----------------------------------------------------
img_proof/__init__.py                4      0   100%
img_proof/collect_items.py          15      0   100%
img_proof/ipa_aliyun.py            136      8    94%
img_proof/ipa_azure.py             223     22    90%
img_proof/ipa_cloud.py             421     65    85%
img_proof/ipa_constants.py          24      0   100%
img_proof/ipa_controller.py         56      1    98%
img_proof/ipa_distro.py             78      4    95%
img_proof/ipa_ec2.py               132     17    87%
img_proof/ipa_exceptions.py         21      0   100%
img_proof/ipa_fedora.py              3      0   100%
img_proof/ipa_gce.py               215     18    92%
img_proof/ipa_oci.py               188     21    89%
img_proof/ipa_opensuse_leap.py       3      0   100%
img_proof/ipa_redhat.py             16      0   100%
img_proof/ipa_rhel.py                3      0   100%
img_proof/ipa_sle_micro.py           3      0   100%
img_proof/ipa_sles.py               16      0   100%
img_proof/ipa_ssh.py                34      2    94%
img_proof/ipa_utils.py             269     25    91%
img_proof/scripts/__init__.py        1      0   100%
img_proof/scripts/cli.py           232     17    93%
img_proof/scripts/cli_utils.py      97      5    95%
----------------------------------------------------
TOTAL                             2190    205    91%

Required test coverage of 90.0% reached. Total coverage: 90.64%
=========================================================================== 221 passed, 2 warnings in 5.93s ============================================================================

[smarlowucf]

I think this can be more simple implementation. At some point in the code I think we end up with a list of tests. If we have a list of test names to exclude we can do a "subtraction".

tests = [test for test in tests where test not in excluded_tests]

[grisu48]

I think this can be more simple implementation. At some point in the code I think we end up with a list of tests. If we have a list of test names to exclude we can do a "subtraction".

tests = [test for test in tests where test not in excluded_tests]

We could simplify the code by also extending the exclusion pattern accordingly:

        if exclude is not None and len(exclude) > 0:
            exclude = ipa_utils.expand_test_files(
                self.test_dirs,
                exclude
            )
            self.test_files = [test for test in self.test_files if test not in exclude]

The caveat is that the tests in the exclusion list must exist otherwise img-proof will fail, however that might be a desirable side-effect. e.g.: --exclude test_sles_IDONTEXIST results in

IpaUtilsException: Test file with name: test_sles_IDONTEXIST cannot be found.

Will update the code accordingly.

[smarlowucf]

I don't think it makes sense to add the option to skip test descriptions. These are easy to drop at invocation as compared to individual test files in a test description.

E.g. if you call img-proof with test_sles and test_sles_gce and want to drop test_sles_gce you just call img-proof with test_sles. If you want to drop an individual test inside a test description then it makes sense because you could call img-proof with test_sles and exclude test_sles_motd from that description.

[grisu48]

I don't think it makes sense to add the option to skip test descriptions. These are easy to drop at invocation as compared to individual test files in a test description.

E.g. if you call img-proof with test_sles and test_sles_gce and want to drop test_sles_gce you just call img-proof with test_sles. If you want to drop an individual test inside a test description then it makes sense because you could call img-proof with test_sles and exclude test_sles_motd from that description.

The intention is to skip certain tests like test_sles_motd and AFAICS this is exactly how it is handled right now. So the intended call is e.g.

img-proof test gce <snip> --exclude test_sles_motd test_sles

This results in all tests from test_sles being executing except for test_sles_motd.

I don't see how test descriptions play a role here (assuming test_sles is the test name, not description).

[smarlowucf]

Right, we don't need to expand the exclude arg though. Thus it can be simplified to:

if exclude:
    self.test_files = [test for test in self.test_files if test not in exclude]

Because Python treats an empty list as "nonesy" so there's no need to check the length.

[grisu48]

Please check again. pytest --cov=img_proof is still passing.

[smarlowucf]

Oh I see, I really don't like the idea of enabling exclude on test descriptions as opposed to individual test files but we do need to expand the short test name to the file name :thinking: . Let me ponder this.

[grisu48]

Oh I see, I really don't like the idea of enabling exclude on test descriptions as opposed to individual test files but we do need to expand the short test name to the file name thinking . Let me ponder this.

Of course. From a user perspective I think the desirable state is to have the input for the tests and for the test exclusion the same, otherwise it is confusing. This is now the case - both rely on ipa_utils.expand_test_files.

[smarlowucf]

Okay, I looked into this some more. There's a simpler solution that prevents us from parsing twice by doing the exclude check in parse_sync_points. I can cherry pick changes into a new branch but don't want to stomp on your contributions. Basically we can do it like:

def parse_sync_points(names, tests, exclude):
...snip...

    for name in names:
        if name in exclude:
            continue
       elif name in SYNC_POINTS:
            test_files.append(name)
        else:
            test_files.append(find_test_file(name, tests))

    return test_files

[github-actions[bot]]

Ping, pull request has no activity for the last 7 days.

[rjschwei]

exclude added with c2d71f66