Refactor tests

[RBrearton]

I noticed that the tests made extensive use of global variables, wildcard imports, multiple assertions per test, and generally looked in need of some work.

I replaced all global variables with pytest fixtures, removed all wildcard imports, updated requirements.txt to reflect the dependency on pytest (and the lazy_fixtures plugin which is arguably essential, see e.g. pytest-dev/pytest#349). With regards to multiple assertions, I only touched a couple of tests that could be refactored easily.

Functionally, this refactor changes nothing, but at least now you know that no two tests can affect each other's state thanks to fixtures.

[rayosborn]

This looks to be OK. I have two slight concerns. One is that the use of fixtures to define the two types of empty NXdata groups, i.e., NXdata_from_empty_01 and NXdata_from_empty_02, makes it a little harder to understand what the tests are doing. I think it is more important that the test code is easy to understand than that it is more efficient. I'm also worried that adding a dependency on pytest-lazy-fixture might cause problems for others who forked the repository and it doesn't look particularly actively maintained. Do we know how stable it is?

[rayosborn]

I wonder if it might be better to use request.getfixturevalue, which may be slightly less elegant, but is a part of the standard pytest package, assuming https://miguendes.me/how-to-use-fixtures-as-arguments-in-pytestmarkparametrize is correct.

[RBrearton]

On the second count I agree: I've always just used lazy_fixtures because, well, I'm lazy, and the package very quickly does what it says on the tin! I'll commit a version that doesn't depend on pytest-lazy-fixtures, I've seen people use request.getfixturevalue before, I've just never used it myself.

As for the use of fixtures to define the [empty] groups, I felt that this is easier to follow (even if more verbose). Whenever I've worked with pytest in the past, people have been quite religious about keeping all logic pertaining to the initialisation of objects in external fixtures. Then the bodies of the test functions contain exclusively calls to the methods we would like to test, and assertions. But, given that there aren't so many tests at the moment, I don't mind either way; certainly it will be fewer lines of code to not use fixtures for them!

[rayosborn]

That sounds good. Keep the fixtures as they are, but if you can swap the requests.getfixturevalue for pytest-lazy-fixtures, then I'll be happy to merge this. Thanks for your work on this.

[RBrearton]

@rayosborn I've pushed an update replacing all lazy_fixture calls with the equivalent request.getfixturevalue, and removed pytest-lazy-fixture from the requirements.txt

[rayosborn]

This looks good to merge.