How to switch to pytest?

[mih]

We had mixed interest in the discussion about this #4090, but with #6211 it is no abundantly clear that this is no longer an academic exercise.

I am quite confident that such a switch is no side-hustle for a single dev. In all likelihood we have to organize a sprint with a clear objective and approach, and then make it happen in one go.

The only real attempt to do anything about this was #4232

[yarikoptic]

I am afraid I agree -- sprint would be nice, but since use of our decorators is so much intertwined through our tests, it might be even "difficult to start" in a sprint fashion. I think what would be worth for some singular developer to approach our decorators and either make them into pytest fixtures, but another approach (more lightweight but not universal) IIRC was just to keep them, and to make args into kwargs in the tests, see e.g. use of @with_tree within reproman code which is pytest'ed
https://github.com/ReproNim/reproman/blob/HEAD/reproman/tests/test_utils.py#L317

@jwodder , since you have extensive knowledge of py.test , could you please give an initial shot to e.g.

• convert our module wide nose fixtures to pytest (https://github.com/datalad/datalad/blob/master/datalad/__init__.py#L102 )
• see how https://github.com/datalad/datalad/blob/master/datalad/tests/test_tests_utils.py tests and actual helpers in https://github.com/datalad/datalad/blob/master/datalad/tests/utils.py need to be adjusted or converted to pytest fixtures

? I think based on the success/lessons learned from this "small" exercise we might then work out a more informed plan for migration? (and that work shouldn't conflict with anything ongoing, as if we e.g. started to convert all the tests)

[jwodder]

@yarikoptic

• Should this branch off of master or maint?
• Do you have any preference about to what extent the syntax of the helpers should be preserved?

[mih]

My 2ct: I think it should be off master for the test ballon, and if successful implemented immediately following a release (where there is minimal diff between the two). It will cause major disruption anyways. Not even thinking about the old PRs that are dangling here....

[jwodder]

I'm in the process of replacing the use of _test_states in setup_package()/teardown_package() with pytest's monkeypatching, and I've encountered this line in teardown_package():

datalad/datalad/__init__.py

Lines 259 to 260 in 3bfdaab

	if os.environ.get('DATALAD_TESTS_NOTEARDOWN'):
	return

In the current code on master, setting DATALAD_TESTS_NOTEARDOWN means that the environment variables, log level, and consts.DATASETS_TOPURL are not restored to their original values after testing; do you want this to continue to be the case? I can make it so that setting the envvar just leaves the test values of the ui backend, test HTTP server, temp paths, and cfg in place after testing, but if you want the test-local envvars to maintain their test values afterwards when DATALAD_TESTS_NOTEARDOWN is set, it means not fully using pytest's goodness.

[jwodder]

@yarikoptic @mih I would like some input on what the desired syntax for @with_tree and similar decorators should be under pytest. So far, I see the following options:

• Continue using @with_tree as a decorator as is currently done, but make the corresponding test function arguments into keyword arguments, as reproman does. (Apparently, this works? I can't find any spot in pytest's documentation that says it's OK with this.)

• Keep the decorator and test function syntax as-is, but make the decorator implementations perform signature-manipulation shenanigans on each test function so that pytest doesn't see the "tree argument" to the test function. This would also make it possible for @with_tree and other decorators to receive pytest fixtures like tmp_path_factory, thereby letting pytest take care of the temp files etc.

• Make all of the @with_* decorators into pytest fixtures. Note that this means that the name of the test function argument for each fixture must always be the same. For argumentless decorators like @with_memory_keyring, the conversion is easy. For decorators that take arguments like @with_tree, I see the following options:

  1. Follow the pattern described at https://docs.pytest.org/en/6.2.x/fixture.html#using-markers-to-pass-data-to-fixtures so that a typical use of @with_tree becomes something like:

     @pytest.mark.tmptree(tree={
         '.git': {
             '1': '2'
         },
         'd1': {
             '.git': 'possibly a link from submodule'
         },
         'git': 'just a file'
     })
     def test_func(tmptree):
         ...
     

     This will only work if no test function uses more than one of the same @with_* decorator.

  2. Follow the pattern described at https://docs.pytest.org/en/6.2.x/fixture.html#factories-as-fixtures so that a typical use of @with_tree becomes something like:

      def test_func(tmp_tree_factory):
          tmptree = tmp_tree_factory(tree=...)
          ...
     

     The benefit of this pattern is that supports test functions that currently use multiple instances of the same @with_* decorator.

  3. Implement (i) in terms of (ii) so that those test functions that only use a single instance of a given @with_* decorator can use it as a fixture while those that need multiple instances can use the factory pattern.

[yarikoptic]

• Continue using @with_tree as a decorator as is currently done, but make the corresponding test function arguments into keyword arguments, as reproman does. (Apparently, this works? I can't find any spot in pytest's documentation that says it's OK with this.)

well, it might indeed be some undocumented feature that pytest tests are still python functions ;)
The tricky part could be to retain the order of arguments added by the decorators into the function, i.e. I don't know/remember how it would work if some pos args are pytest fixtures.

[yarikoptic]

• Make all of the @with_* decorators into pytest fixtures. Note that this means that the name of the test function argument for each fixture must always be the same.

I could be wrong but I believe we might have some cases (largely of @with_tempfile) to provide multiple arguments.

• Keep the decorator and test function syntax as-is, but make the decorator implementations perform signature-manipulation shenanigans on each test function so that pytest doesn't see the "tree argument" to the test function. This would also make it possible for @with_tree and other decorators to receive pytest fixtures like tmp_path_factory, thereby letting pytest take care of the temp files etc

this sounds seductive albeit "dangerous" since we hoped that for the "first stage" we better minimize the diff (keep/provide all the assert_equal etc) if that is possible. Signature manipulation though sounds too ad-hoc, have you done anything like that with pytest?

[jwodder]

@yarikoptic

we hoped that for the "first stage" we better minimize the diff (keep/provide all the assert_equal etc) if that is possible

If we want to keep the uses of assert_equal etc., it looks like we'll have to define our own versions, as those names are specific to nose, and the corresponding assertEqual() etc. in the Python standard library appear to only be available as methods of the TestCase class, not as standalone functions.

Note that there exists a nose2pytest script for converting assert_* calls to pytest assertions.

Signature manipulation though sounds too ad-hoc, have you done anything like that with pytest?

No. The most I've done with signatures has been this manipulation in fscacher and a library for summarizing the arguments a function takes.

[jwodder]

@yarikoptic @mih I still need an answer to this question about how to update @with_tree-style decorators.

[yarikoptic]

If we want to keep the uses of assert_equal etc., it looks like we'll have to define our own versions, as those names are specific to nose, and the corresponding assertEqual() etc. in the Python standard library appear to only be available as methods of the TestCase class, not as standalone functions.

👍

@yarikoptic @mih I still need an answer to this question about how to update @with_tree-style decorators.

I would have first tried along the selected choice I gave in #6225 (comment) :

Continue using @with_tree as a decorator as is currently done, but make the corresponding test function arguments into keyword arguments, as reproman does. (Apparently, this works? I can't find any spot in pytest's documentation that says it's OK with this.)

unless you run into a show stopper -- I think this would be the least intrusive way

[mih]

I think we have switched to pytest successfully enough now that we can close this issue.