[WIP] Refactor test_memory.py as per pytest design.

[kdexd]

Third Phase PR on #411 (Succeeding PR #458)

NOTE: Rebasing / force pushig didn't allow reopening of this PR, hence a new PR has been created as a replacement ( #466 )

[lesteve]

I would rather have topic-based PRs rather than file-based PRs. It does help the review process a lot by focusing on one aspect at a time.

In this particular case having a PR that just removes all the yield from all the tests would be great. If that means replacing all the yield by just assert without trying to use parametrize I am more than fine with this.

Once this is done, remember to remove disable-warnings from setup.cfg.

[kdexd]

In this particular case having a PR that just removes all the yield from all the tests would be great.

@lesteve This includes only tests of test_memory.py ? Because there are still 112 pytest warnings undone, and yield based tests of other files require parametrize. If yes then I'll search for possibilities in other files and include them in this PR, while shifting the assert_raises and tmpdir related commits in separate branch for a future PR maybe tomorrow...

[lesteve]

@lesteve This includes only tests of test_memory.py ?

When I say all the tests I meant all the tests from all the files.

yield based tests of other files require parametrize.

Do you have an example? Technically I do not think they require parametrize. Using parametrize is just a nicer way of writing the tests, although this is not always the case IMHO. What I was proposing is to just do a straightforward replacement of yield by assert. Using parametrize can be done later.

All I am trying to say is that I would do it in this order:

• remove yield from the tests across the whole codebase (+remove disable-warnings from setup.cfg)
• fix Nested parallel warnings test failing from time to time in python 3 #413. It is not great when "temporarily" skipping a test lasts too long
• Try to write the tests in a nicer way using pytest functionalities. I am fine going back to a file-based approach at this point if this is more convenient for you

[kdexd]

I am keeping this PR on a hold for a while, not doing yield => assert in ALL files here itself, I can change everything but not the name of branch.. When I deal with using parametrize in this file, I will use this PR and get it merged. Will reopen it in a while. Thanks for a clear roadmap 😄

[lesteve]

Thanks for trying to keep the github tracker tidy! Keeping the PR open would have been fine too, to be perfectly honest.

[kdexd]

@lesteve yeah the tracker remains clean, plus I do this because if a third person comes here, he may get a clue that the PR was closed due to a change of plan and reopened later for a related task 😁

[kdexd]

Rebasing / force pushing does not allow this PR to be reopened, I will make a separate one.