Migrate to pytest #1149

Glandos · 2023-02-25T11:29:37Z

Well for now, this is just experiment, but showing good signs of success.

for import test, extract a local fixture

Glandos · 2023-02-26T10:17:35Z

Here it is!
Flask-Testing is now removed, we will be able to upgrade everything else! And write more pytest-esque tests.

zorun · 2023-02-26T11:18:29Z

Thanks for working on this.

What's the rationale for moving to pytest-style tests? Honestly, I find pytest fixtures a bit confusing.

From what I understand, pytest understands unitttest-style tests, so we can just keep using that?

Btw, does pytest have native support for MagicMock and patch?

Glandos · 2023-02-26T14:47:22Z

Ah, we need an explanation/discussion/debate. Good!
Disclaimer: I like the pytest pythonic approach of writing tests. This is purely subjective, so I think it's good to say it before reading.

fixtures are reusable setUp()/teardown(). You just write some code in a function (the setUp), then yield (the value, or nothing), then some code (the teardown). And you can write small functions, they all contains, in one go, their setUp/teardown phases. This is a huge win for me.

Now, the main disadvantage for me are the following implicit behavior:

Globally available fixtures must be in a file called conftest.py. It took me an hour to find it.
Using fixtures can be automatic, based on the test function argument names. E.g., a function test_my_client(app, client) will try to use fixtures called app and client. This can be confusing.

Honestly, for now, I took the lazy approach: don't modify every existing tests, since they use a class and self everywhere. So I declared some small fixtures (to make them a bit independent), and use them wherever needed.

MagickMock and patch: as far as I know, nothing like this exist in pytest. They have a module for monkeypatching, but the one from unittest is far more useful in my opinion, so I'll stick to that. Since unittest is part of standard library, we are free to use when we want to.

zorun · 2023-07-13T16:08:40Z

Yeah, I don't like implicit fixtures. If we do move to pytest, we should document/enforce that we only use explicit fixtures (like the code you propose here)

I don't understand why you need to wrap some test code with a with self.app.app_context():, can you explain?

almet

Hey, looks good to me! I'm glad to see the tests are moving a bit. I guess they could be refactored a lot and reworked to be simpler now.

I approve the changes. I've a few comments about the docs, because I feel it would be clearer this way. Happy to go use pytest now rather than the old unittest way.

almet · 2023-07-14T16:27:13Z

docs/contributing.md

@@ -144,6 +144,18 @@ If you are introducing a new feature, you need to either add tests to
 existing classes, or add a new class (if your new feature is
 significantly different from existing code).

+#### Old tests and new tests


Nit: you need a newline before the paragraph starts.

almet · 2023-07-16T12:08:28Z

docs/contributing.md

+possible to pytest fixtures. However, a lot of tests were already written,
+and are still using class-based tests, albeit without setUp()/teardown().
+
+If you write new tests, please **try to write them with pytest in mind**:


Seems to be missing a newline here as well.

almet · 2023-07-16T12:08:48Z

docs/contributing.md

+Historically, tests were written with unittest and Flask-Testing.
+Because the latter was dying, we removed it and switch as much as
+possible to pytest fixtures. However, a lot of tests were already written,
+and are still using class-based tests, albeit without setUp()/teardown().


put the method names within backticks for formatting.

almet · 2023-07-16T12:09:40Z

docs/contributing.md

+and are still using class-based tests, albeit without setUp()/teardown().
+
+If you write new tests, please **try to write them with pytest in mind**:
+- standalone function (not in class)


Maybe a sentence would be better, like "tests are done in standalone functions rather than in class and class methods".

almet · 2023-07-16T12:10:52Z

docs/contributing.md

+
+If you write new tests, please **try to write them with pytest in mind**:
+- standalone function (not in class)
+- plain asserts


Use plain assert rather tha this.assertTrue, for instance.

almet · 2023-07-16T12:13:41Z

docs/contributing.md

@@ -144,6 +144,18 @@ If you are introducing a new feature, you need to either add tests to
 existing classes, or add a new class (if your new feature is
 significantly different from existing code).

+#### Old tests and new tests
+Historically, tests were written with unittest and Flask-Testing.


Rather than starting by explaining how it was in the past, it might be better to explain how it's supposed to be now, and then explain why we have legacy code around. Maybe something like :

Tests are now using pytest. However, [...].

almet · 2023-07-16T12:14:48Z

ihatemoney/tests/budget_test.py

@@ -119,6 +119,7 @@ def test_invite(self):
        resp = self.client.get("/raclette/join/token.invalid", follow_redirects=True)
        self.assertIn("Provided token is invalid", resp.data.decode("utf-8"))

+    @pytest.mark.usefixtures("app_ctx")


Is there a way to avoid repeating this line every so often?

I would recommend having a look at pytest-flask. I think this is not documented, but this allows among other things to access the flask app context directly in the tests (without with app.app_context(): ...

almet · 2023-07-16T12:16:05Z

ihatemoney/tests/conftest.py

+        yield
+
+
+@pytest.fixture


I'm not sure if it's possible with pytest, but any chance this could be in the same file as the tests?

conftest.py is the place for shared fixtures, but pytest allows to define fixtures in test files 👌

azmeuk · 2023-07-28T14:43:00Z

ihatemoney/tests/common/ihatemoney_testcase.py



-class BaseTestCase(TestCase):
+@pytest.mark.usefixtures("client", "converter")
+class BaseTestCase(unittest.TestCase):


Inherinting from unittest.TestCase is not required with pytest.

https://docs.pytest.org/en/7.1.x/getting-started.html#group-multiple-tests-in-a-class

However requires following the class naming convention (start test class names with Test) and abandon unittest assertions self.assert*. For this unittest2pytest can be useful.

azmeuk · 2023-07-28T15:00:54Z

Any reason why sticking to self.assert* unittest assertions? Nowadays pytest assertions produce legible error messages, are easier to write and easier to read, though I know not everybody shares this opinion in the python community. Is this a unit test style choice in IHM or do you plan to move on in a second step?

almet · 2023-07-28T15:38:34Z

No, there are no reason. If we are to decide to go the pytest way, I believe it's just a migration we have to do, but which is not done yet.

@zorun, you seem to be a against using pytest. Would that be a big problem for you? Is it mainly a problem with fixtures?

On my side, I don't have a strong opinion about this. I like pytest and have been using it in other projects without trouble. I would feel more confortable with it, but I can also keep things as they currently are.

zorun · 2023-07-28T16:06:56Z

I'm not strongly opposed to pytest, I just don't like implicit fixtures. If we can avoid them, then let's move to pytest.

The other thing I find really strange is the need for the app_context sometimes, but @azmeuk mentioned that pytest-flask should solve this point.

More work for @Glandos I guess :D

azmeuk · 2023-07-28T16:11:15Z

More work for @Glandos I guess :D

I can gladly help with the pytest migration if ever you don't have a lot of time to spend @Glandos.

zorun · 2023-10-01T21:34:34Z

Superseded by #1213, thanks @Glandos for the initial work!

Glandos added 6 commits February 25, 2023 12:28

first tests with pytest

b7ef23f

use more app context

b84f63e

for import test, extract a local fixture

format

4d1415f

goodbye flask-testing

8c8dfbb

goodbye flask-testing (2)

accfe07

update test contribution doc

b1ed044

Glandos marked this pull request as ready for review February 26, 2023 10:11

Glandos requested review from almet, symphorien and zorun February 26, 2023 10:18

a bit more comment

8dd0c9f

Glandos mentioned this pull request Jul 13, 2023

Prepare next release #1179

Merged

almet mentioned this pull request Jul 16, 2023

Migrate existing sessions after conversion to dict #1194

Merged

almet approved these changes Jul 16, 2023

View reviewed changes

azmeuk reviewed Jul 28, 2023

View reviewed changes

azmeuk mentioned this pull request Aug 12, 2023

pytest migration #1213

Merged

zorun closed this Oct 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate to pytest #1149

Migrate to pytest #1149

Glandos commented Feb 25, 2023

Glandos commented Feb 26, 2023

zorun commented Feb 26, 2023

Glandos commented Feb 26, 2023

zorun commented Jul 13, 2023

almet left a comment

almet Jul 14, 2023

almet Jul 16, 2023

almet Jul 16, 2023

almet Jul 16, 2023

almet Jul 16, 2023

almet Jul 16, 2023

almet Jul 16, 2023

azmeuk Jul 27, 2023

almet Jul 16, 2023

azmeuk Jul 27, 2023

azmeuk Jul 28, 2023 •

edited

azmeuk commented Jul 28, 2023

almet commented Jul 28, 2023

zorun commented Jul 28, 2023

azmeuk commented Jul 28, 2023

zorun commented Oct 1, 2023

		yield


		@pytest.fixture

Migrate to pytest #1149

Migrate to pytest #1149

Conversation

Glandos commented Feb 25, 2023

Glandos commented Feb 26, 2023

zorun commented Feb 26, 2023

Glandos commented Feb 26, 2023

zorun commented Jul 13, 2023

almet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

azmeuk Jul 28, 2023 • edited

Choose a reason for hiding this comment

azmeuk commented Jul 28, 2023

almet commented Jul 28, 2023

zorun commented Jul 28, 2023

azmeuk commented Jul 28, 2023

zorun commented Oct 1, 2023

azmeuk Jul 28, 2023 •

edited