Convert python test suite to pytest

[craigds]

(Spun off from this comment #947 (comment)

GDAL's python test suite is a little bit crazy, and in need of an overhaul.

We've been using pytest lately with great success. If there's interest in this I could probably work on something to convert the GDAL test suite to use it.

To give an idea of the pytest style, consider this test

As a strawman, I'd probably rewrite that something like this using pytest:

@pytest.fixture()
def gdaladdo():
    """
    Passes the gdaladdo path to the test.
    If it's None, all tests relying on it will be skipped.
    """
    gdaladdo_path = test_cli_utilities.get_gdaladdo_path()
    if gdaladdo_path is None:
        raise pytest.skip("gdaladdo wasn't found")

    def run_gdaladdo(arg_string):
        # Hides this frame from tracebacks if it errors,
        # so the context you actually _see_ will be from the test function.
        __tracebackhide__ = True

        # Run the tool.
        # Let all the output just go to stdout/stderr.
        # (pytest swallows the output if the test passes.)
        # Just let it error if the command fails.
        subprocess.check_call(
            [gdaladdo_path] + shlex.split(arg_string),
        )

    return run_gdaladdo


# A magic helper I'd invent which copies certain files to a temp directory.
# Does os.chdir() to that directory just for the test.
# Removes the directory and all its contents afterwards.
@pytest.mark.require_files('gcore/data/mfloat32.vrt', 'gcore/data/float32.tif')
def test_gdaladdo_1(gdaladdo):
    # The 'gdaladdo_path' arg comes from the fixture defined above.
    # We can do all sorts of flexible things like this.

    gdaladdo('gcore/data/mfloat32.vrt 2 4')
    assert os.path.exists('gcore/data/mfloat32.vrt.ovr')

    # I'm assuming we can change these helpers to use `assert` with useful error messages,
    # so if they fail, the test fails.
    tiff_ovr.tiff_ovr_check(gdal.Open('mfloat32.vrt'))

That's quite a bit of refactoring, so doing that to the entire test suite is a largish goal. I also doubt much of that could be automated easily.

However, I think we could very easily make a compatibility shim to make the old tests work with it in the meantime. Something like this:

def old_test(testfunc):
    """
    Compatibility hook for old tests to make them function with pytest.
    """
    @wraps.testfunc
    def wrapper():
        __tracebackhide__ = True
        try:
            ret = testfunc()
        if ret == 'skip':
            pytest.skip(gdaltest.reason or '')
        elif ret == 'fail':
            assert False, gdaltest.reason or ''

    return wrapper


@old_test
def test_gdaladdo_1():
    ...

In fact, we could even override a hook in the pytest configuration file to wrap all the tests with this, but I suspect it will be nice having tests wrapped explicitly with it rather than magically. That way you can see how many old tests remain by just grepping for it.

If this all makes sense, I can probably whip up a PR for the initial compatibility step.

[rouault]

Thanks for all those good ideas. I don't have experience with pytest, but that sounds like a reasonable solution, and some prototyping to get a sense of it would be good.

• Would we have the capability of running tests per file (best we you know your current work is isolated) or the whole test suite as currently ?
• For the compatibility shim, would we have the stacktrace at the point of gdaltest.post_reason() as currently ?
• Are you confident we could have the same autotest/ code as currently for both python2 & python3 with this testing framework ?
• we have a few crazy tests like autotest/gcore/gdal_api_proxy.py where the main test forks a new Python interpreter in another process. Do you think such things could still be made ? (I guess so, but the way we check for the result of the subprocess execution, if ret.find('Failed: 0'), would likely to have been changed)

[rcoup]

Craig's going to bring it up on gdal-dev when he's back from leave next week, but I can probably answer most of these:

Would we have the capability of running tests per file (best we you know your current work is isolated) or the whole test suite as currently ?

Yes. You can have pretty advanced test selectors. eg:

$ pytest path/to/                     # all tests in directory
$ pytest path/to/test.py              # specific file
$ pytest path/to/test.py::test_func   # specific test
$ pytest --last-failed                # the tests that failed last time
$ pytest -m 'internet'                # specific test tag
$ pytest -m 'not geopackage'          # exclude specific test tag

Tags are pretty powerful, can mark tests with anything and then use that to include/exclude sets. There's also the usual skip, skip-if, and expected-fail handling on top of that.

For the compatibility shim, would we have the stacktrace at the point of gdaltest.post_reason() as currently ?

Python supports re-framing tracebacks, so between that and pytest's custom assertions I think it will be good. We could also use custom assertions to do eg. consistent image/metadata/etc comparisons across the test suite.

Since pytest collapses output by default for successful tests so they can print/log in much more detail, I think the gdaltest.reason stuff could [eventually] go away in a lot of cases since the assertion would provide the "where".

Enabling a custom gdal log handler for tests so we can set CPL_DEBUG/CPL_CURL_VERBOSE/etc on automatically for most tests might be helpful too? Means it's automatically on hand when tests fail. (though I'm debugging a CPL_DEBUG-only segfault at the moment, so might not always be helpful!)

Are you confident we could have the same autotest/ code as currently for both python2 & python3 with this testing framework ?

pytest supports both py2 & py3, so there's no reason we can't support both. This is a different idea from eg. tox which runs your test suite under different python versions — I can't see a lot of benefit for GDAL for something like that except making sure SWIG is doing its job? But tox & pytest can work together if there is specific python compatibility we want to test.

we have a few crazy tests like autotest/gcore/gdal_api_proxy.py where the main test forks a new Python interpreter in another process. Do you think such things could still be made ? (I guess so, but the way we check for the result of the subprocess execution, if ret.find('Failed: 0'), would likely to have been changed)

Sure, at work we have tests that fork subprocesses and start/stop servers and all sorts of crazy stuff. If we can wrap it in a function that checks the return code/output/etc it should be good.

[rouault]

$ pytest path/to/test.py # specific file

Note that GDAL tests assume that they are run with the current working directory being the one where the file is located. Due to relative test filename. The pymod/gdaltest.py infrastructure makes sure to change the CWD automatically.

I think the gdaltest.reason stuff could [eventually] go away in a lot of cases since the assertion would provide the "where".

I agree. Having to write gdaltest.post_reason("some painful message"); return "fail" is the worse part of the current testing experience (I'm super lazy and in general my reason is "fail", and I just look at the line number in the report to figure out where, and then why, it failed). My remark was just to make sure that for tests not upgraded and using the compatibility shim we would still get this line number because the "fail" reason might not be sufficient to locate the precise point of failure.

pytest supports both py2 & py3

My question was to make sure that there wasn't pytest API/constructs that was different in py2 & py3 so we can have the same autotest code. Testing py2 vs py3 is needed since we have SWIG typemaps that are fairly different in both cases (string vs byte array handling of course...)

[rcoup]

I agree. Having to write gdaltest.post_reason("some painful message"); return "fail" is the worse part of the current testing experience (I'm super lazy and in general my reason is "fail", and I just look at the line number in the report to figure out where, and then why, it failed).

By default assert a == b will raise a failure, print a representation of a & b, give you a traceback, and print out the surrounding context lines

You can also do assert a == b, "Some informative message" which can easily add a little more information for non-obvious fails.

Testing py2 vs py3 is needed since we have SWIG typemaps that are fairly different in both cases (string vs byte array handling of course...)

Sure, so in CI we could test under both interpreters just by running pytest using each one. Not sure most developers need to run all tests against both python versions during dev though? We can skip tests based on python interpreter version if there's particular oddities we're testing.

Note that GDAL tests assume that they are run with the current working directory being the one where the file is located. Due to relative test filename. The pymod/gdaltest.py infrastructure makes sure to change the CWD automatically.

Sure, so the compat wrapper could fix that by changing cwd as part of a fixture.

[rouault]

Sure, so in CI we could test under both interpreters just by running pytest using each one.

In the current CI, we have a dedicated config to run with Py3.

Not sure most developers need to run all tests against both python versions during dev though?

No, generally the current python version is good enough. But it is good to be able to "python my.test" or "python3 my.test" manually if needed (that involves tweaking the PYTHONPATH when working with a dev non-installed environment)

[craigds]

An issue I just thought of: pytest supports python 2.7+.

I see compatibility code for Python 2.3 in GDAL (!)

Is there any reason to keep this for GDAL 2.4+? Python 2.6 has been unsupported for years now...

If not, I love ripping things out 😄

For the compatibility shim, would we have the stacktrace at the point of gdaltest.post_reason() as currently ?

Almost all of the calls to post_reason() appear to be of the form:

    if condition:
       gdaltest.post_reason(reason_message)
       return 'fail'

I think it would be straightforward to automate turning that into:

assert not condition, reason_message

pytest gives a stack trace at the assertion point including line numbers, stdout/stderr, warning messages and relevant context variables, which in addition to the reason message, is generally plenty to diagnose the issue.

Perhaps the remaining calls to post_reason will be few enough to remove manually.

[rouault]

Is there any reason to keep this for GDAL 2.4+? Python 2.6 has been unsupported for years now...

As far as I now, none of our CI targets tests 2.6 or older. I presume the bindings themselves can support older python, but for me requiring 2.7+ is fine at that point.

[craigds]

A naïve first pass at auto-translating those, converted 7076/14487 of the gdaltest.post_reason calls to asserts. I'm fairly confident I could boost that to 90% with a small amount of work.

update: 12139/14487 now (84%. nearly there already!)

[craigds]

I've posted this to gdal-dev