You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@airflow.apache.org by Kamil BreguĆa <ka...@polidea.com> on 2020/01/01 19:10:56 UTC
Test style convention after migration to pytest
Hello,
We have recently migrated to pytest. Code written according to the
standard library - unittest.TestCase is still compatible with pytest.
The AIP-21 document did not discuss the migration of current code to
new features, but only discussed the change of runner and benefits of
pytest over nosetets.
Link:
https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-27+Migrate+to+pytest
As far as I appreciate the many advantages of using this tool, but I am not
sure **whether, how or when we want to start using some features**. I
prefer to keep the current project conventions in many areas, but I still
love pytest. I think we should establish general conventions on writing
tests and recommended solutions to known problems. I prefer a pragmatic
approach, not just the use of features, because now we can use it.
Unfortunately, I do not see many benefits from new features.
I would not like the code to have many ways of expressing the same
solution. For the following reasons:
* it can introduce a lack of understanding among new contributors
* facilitate the understanding and reading of code.
* creates unnecessary discussion about the preferences of one way over the
other. Not related to changes.
* forces an understanding of the complex syntax of some solutions.
* encourages people to rewrite the code, which can generate unnecessary
work.
To establish a full convention, I have prepared a few questions:
1) Do we want to use additional plugins when there is no function in the
standard library e.g. flaky marker, forked marker? This is, in my opinion,
one of the big advantages of migrating to pytest.
2) Do we want to use custom markers?
The discussion takes place in a separate thread:
https://lists.apache.org/thread.html/4538437c96f599766005ba7829d0bee1511debb4f53599e0d300a56f%40%3Cdev.airflow.apache.org%3E
3) Do we want to use new assert statement?
https://lists.apache.org/thread.html/08b64d3b084c865399f98f6c6f56235ce5329e843d97938e1a8045a5%40%3Cdev.airflow.apache.org%3E
Based on the discussion with devlist, we want to delay migrations to the
new assert statement.
4) Do we want to remove inheritance from unittest.TestCase?
5) Do we want to use class-less tests?
6) Do we want to use pytest function instead of current?
For example:
Unittest.assertRaises vs pytest.raises
parametrize vs pytest.mark.parametrize
unittest.skip[If], vs pytest.mark.skip[If]
7) Do we want to use monkeypatch fixture?
https://docs.pytest.org/en/latest/monkeypatch.html
8) Do we want to use the pytest fixtures?
Do we want to migrate all code to pytest fixtures?
We are currently using a different style of fixtures. Do we want to give it
up?
https://docs.python.org/3/library/unittest.html#class-and-module-fixtures
It is worth asking to think about whether we do not want to change this
convention in the future e.g. after dropping support for 1.10.X. We can
also allow two conventions on a temporary basis, and then migrate to one at
a later time. For example, we want to migrate to the assert statement after
dropping support for 1.10
I hope I found the main differences between the current convention and the
new convention. However, if you missed something, please continue to number.
Best regards,
Kamil
Re: Test style convention after migration to pytest
Posted by Ash Berlin-Taylor <as...@apache.org>.
đđ»
On 10 January 2021 12:18:28 GMT, Tomasz Urbaszek <tu...@apache.org> wrote:
>Hello all!
>
>The PR for changing self.assert to plain assert is here:
>https://github.com/apache/airflow/pull/12951
>
>All tests seem to be passing (except on heisentest) so I'm going to merge
>on Monday if there are no objections.
>
>Cheers,
>Tomek
>
>On Thu, Oct 22, 2020 at 1:04 PM Ash Berlin-Taylor <as...@apache.org> wrote:
>
>> 2. We already do don't we for backend and integrations etc?
>> 3. I am strongly in favour of -- using assert with pytest gives much
>> nicer error messages than unittest's own assertions;
>> 4. if we do 3, then I'm also in favour of doing 4 as we don't need it
>> anymore, and we can then use more plugins/features of pytest (parameterized
>> behaves better I think)
>> 5. No view for or against
>> 6. depends on what we do for 4
>> 7. unittest.mock can be confusing to use, but I'm not sure pytests
>> monkeypatch fixture is nicer. No opinoin on this one
>> 8. I like them, but the automatic dependency injection can be confusing
>> until you are familar with Pytest. An example of them for anyone unaware:
>> https://github.com/apache/airflow/pull/11694 (look at the example test at
>> the end of the PR description)
>>
>> I don't think we need to (yet) do any bulk changes to the test, but if we
>> agree that using more of pytest is the way to go then we can write new
>> tests using assert style, and once we've got a good chunk of the way we can
>> bulk switch the rest.
>>
>> -ash
>>
>> On Oct 22 2020, at 11:45 am, Tomasz Urbaszek <tu...@apache.org> wrote:
>>
>> Hi all,
>>
>> I would like to resurrect this discussion as another one was started in
>> https://github.com/apache/airflow/issues/11657#issuecomment-713834942
>>
>> Main questions were:
>>
>> > 1) Do we want to use additional plugins when there is no function in
>> > the standard library e.g. flaky marker, forked marker?
>> > 2) Do we want to use custom markers?
>> > 3) Do we want to use new assert statement?
>> > 4) Do we want to remove inheritance from unittest.TestCase?
>> > 5) Do we want to use class-less tests?
>> > 6) Do we want to use pytest function instead of current?
>> > 7) Do we want to use monkeypatch fixture?
>> > 8) Do we want to use the pytest fixtures?
>>
>> Probably any changes we decide on should be enforced after 2.0.
>>
>> Cheers,
>> Tomek
>>
>> On 2020/01/18 01:08:29, Kamil BreguĆa <ka...@polidea.com> wrote:
>> > Hello,
>> >
>> > Sorry for the late reply. Recently, I have been very busy with the
>> > client project and work on the AIP-21.
>> >
>> > Let's start. I split this message into several sections for easier
>> reading.
>> >
>> > **Establishment of a convention**
>> > I will prepare a summary. It is available here:
>> >
>> >
>> https://docs.google.com/spreadsheets/d/10qToeTFMxjShBX3xcK6IWr9Sp8Pw7qk35fQTtzpZYYs/edit?usp=sharing
>> >
>> > The mailing list does not allow rich content, so I copy the verdicts.
>> >
>> > 1) Do we want to use additional plugins when there is no function in
>> > the standard library e.g. flaky marker, forked marker?YES
>> > 2) Do we want to use custom markers? YES
>> > 3) Do we want to use new assert statement? YES
>> > 4) Do we want to remove inheritance from unittest.TestCase? YES
>> > 5) Do we want to use class-less tests? NO
>> > 6) Do we want to use pytest function instead of current? No verdict
>> > 7) Do we want to use monkeypatch fixture? NO
>> > 8) Do we want to use the pytest fixtures? YES
>> >
>> > We don't have a verdict in question 6, because we have no answer from
>> > Tomek and Jarek. Can you answer this question? A detailed explanation
>> > is available.
>> >
>> https://lists.apache.org/thread.html/7ae7ba0e72d3f0d12f8398a85980c5064b58574caa727c6a974fc628%40%3Cdev.airflow.apache.org%3E
>> > Should we use the pytest functions or use the standard functions?
>> >
>> > **Using the new convention in the current code**
>> > tl;dr; Two conventions will be accepted simultaneously.
>> > Long story: As for the time of using the new convention, I'm the only
>> > one who is worried about whether to allow it now. Other people did not
>> > share my fears. So I think we should allow the use of the new
>> > conventions now. We want to automate the introduction of the new
>> > convention in the future, so I think that the application of the two
>> > conventions should be allowed temporarily. However, I will describe
>> > the new conventions, but we do not need to strictly require them until
>> > the remaining code is migrated.
>> >
>> > **Full migration to the new convention**
>> > tl;dr; We'll do it later.
>> > Long story: Should we migrate the remaining code now? Tomek prefers
>> > to wait for the end of work on AIP-21. Kaxil and I also prefer to wait
>> > for AIP-21 or AIrflow 2.0. Jarek prefers to wait until we finish all
>> > other works that improve the development environment e.g. pylint. So
>> > we everybody prefers to wait. I think we can go back to the discussion
>> > when the pylint is completed or we finish AIP-21. It depends on which
>> > will be earlier.
>> >
>> > Best regards,
>> > Kamil
>> >
>> > On Fri, Jan 17, 2020 at 3:07 PM Jarek Potiuk <Ja...@polidea.com>
>> wrote:
>> > >
>> > > Should we resume this :)?
>> > >
>> > > On Mon, Jan 6, 2020 at 3:42 PM Jarek Potiuk <Ja...@polidea.com>
>> > > wrote:
>> > >
>> > > > Good idea!
>> > > >
>> > > > On Mon, Jan 6, 2020 at 3:12 PM Kamil BreguĆa <
>> kamil.bregula@polidea.com>
>> > > > wrote:
>> > > >
>> > > >> Hello,
>> > > >>
>> > > >> Now is the holiday period. Some people have not started working yet.
>> > > >> Others are busy with New Year activities. What do you think about
>> > > >> Friday?
>> > > >>
>> > > >> Best regards,
>> > > >> Kamil
>> > > >>
>> > > >> On Mon, Jan 6, 2020 at 2:48 PM Tomasz Urbaszek
>> > > >> <to...@polidea.com> wrote:
>> > > >> >
>> > > >> > When should we assume that we've reached a consensus?
>> > > >> >
>> > > >> > T.
>> > > >> >
>> > > >> > On Thu, Jan 2, 2020 at 12:52 AM Jarek Potiuk <
>> Jarek.Potiuk@polidea.com>
>> > > >> > wrote:
>> > > >> >
>> > > >> > > > 1) Do we want to use additional plugins? *Yes*
>> > > >> > >
>> > > >> > > 2) Do we want to use custom markers? *Yes. They will help with
>> > > >> optimising
>> > > >> > > > our test execution.*
>> > > >> > >
>> > > >> > > 3) Do we want to use new assert statement? *Yes*
>> > > >> > > > 4) Do we want to remove the inheritance from
>> unittest.TestCase? *No
>> > > >> > > > opinion about it. I am ok with both.*
>> > > >> > > > 5) Do we want to use class-less tests? *No.*
>> > > >> > > > 6) Do we want to use pytest function instead of current? *I
>> don't
>> > > >> > > > understand. Can you explain please?*
>> > > >> > > > 7) Do we want to use monkeypatch fixture? *No. Mock is
>> better.*
>> > > >> > > > 8) Do we want to use the pytest fixtures? *Yes.*
>> > > >> > > >
>> > > >> > >
>> > > >> > > Great that we have this discussion now. I also think we should
>> just
>> > > >> agree
>> > > >> > > it now and not introduce it "globally".
>> > > >> > > Once we do it, we should simply add it together new features we
>> > > >> implement.
>> > > >> > > We have still pylint to finish as a "non-functional global
>> change"
>> > > >> and we
>> > > >> > > should not add new one. It's good to continually improve but one
>> > > >> thing at a
>> > > >> > > time.
>> > > >> > >
>> > > >> > > BTW Pylint goes well we are down to 243 non-pylint files from
>> 991
>> > > >> since
>> > > >> > > May.
>> > > >> > >
>> > > >> > > J.
>> > > >> > >
>> > > >> > > On Thu, Jan 2, 2020 at 12:40 AM Kaxil Naik <kaxilnaik@gmail.com
>> >
>> > > >> wrote:
>> > > >> > >
>> > > >> > > > This is a tough one. Both arguments are reasonable.
>> > > >> > > >
>> > > >> > > > I agree at some point we should convert all to use assert.
>> But at
>> > > >> the
>> > > >> > > same
>> > > >> > > > time, we should also focus on adding *more user-facing
>> features *and
>> > > >> > > spend
>> > > >> > > > less time on more refactor or similar changes.
>> > > >> > > >
>> > > >> > > > So based on that, this might be a low priority. We also need
>> to
>> > > >> still
>> > > >> > > > complete AIP-21
>> > > >> > > > <
>> > > >> > > >
>> > > >> > >
>> > > >>
>> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-21%3A+Changes+in+import+paths
>> > > >> > > > >
>> > > >> > > > which is very critical for 2.0.
>> > > >> > > >
>> > > >> > > > 1) Do we want to use additional plugins?
>> > > >> > > >
>> > > >> > > >
>> > > >> > > > Yes.
>> > > >> > > >
>> > > >> > > > 2) Do we want to use custom markers?
>> > > >> > > >
>> > > >> > > >
>> > > >> > > > Not sure yet. Low Priority for me.
>> > > >> > > >
>> > > >> > > > 3) Do we want to use new assert statement?
>> > > >> > > >
>> > > >> > > >
>> > > >> > > > I think new PRs can contain it, shouldn't be a problem as
>> long as
>> > > >> it is
>> > > >> > > > documented to avoid confusion.
>> > > >> > > >
>> > > >> > > > 4) Do we want to remove the inheritance from
>> unittest.TestCase?
>> > > >> > > >
>> > > >> > > >
>> > > >> > > > Yes, this is, however, going to change how people write
>> tests. So
>> > > >> someone
>> > > >> > > > has to own it as it can become painful with PRs getting merged
>> > > >> > > > continuously.
>> > > >> > > >
>> > > >> > > > 5) Do we want to use class-less tests?
>> > > >> > > >
>> > > >> > > >
>> > > >> > > > No.
>> > > >> > > >
>> > > >> > > > 6) Do we want to use pytest function instead of current?
>> > > >> > > >
>> > > >> > > >
>> > > >> > > > Yes
>> > > >> > > >
>> > > >> > > > 7) Do we want to use monkeypatch fixture?
>> > > >> > > >
>> > > >> > > >
>> > > >> > > > I also prefer unittest.mock but open to suggestions.
>> > > >> > > >
>> > > >>
>> https://github.com/pytest-dev/pytest/issues/4576#issuecomment-449864333
>> > > >> > > > has
>> > > >> > > > some good comparison on it
>> > > >> > > >
>> > > >> > > > 8) Do we want to use the pytest fixtures?
>> > > >> > > >
>> > > >> > > >
>> > > >> > > > Yes
>> > > >> > > >
>> > > >> > > > On Wed, Jan 1, 2020 at 8:07 PM Kamil BreguĆa <
>> > > >> kamil.bregula@polidea.com>
>> > > >> > > > wrote:
>> > > >> > > >
>> > > >> > > > > @unittest.skip("demonstrating skipping")
>> > > >> > > > >
>> > > >> > > > > On Wed, Jan 1, 2020 at 8:37 PM Tomasz Urbaszek <
>> > > >> turbaszek@apache.org>
>> > > >> > > > > wrote:
>> > > >> > > > > >
>> > > >> > > > > >
>> > > >> > > > > > > 6) Do we want to use pytest function instead of current?
>> > > >> > > > > >
>> > > >> > > > > > I do not understand this point, can you explain?
>> > > >> > > > > >
>> > > >> > > > >
>> > > >> > > > > Pytest Introduces solutions that replace solutions that are
>> now
>> > > >> used
>> > > >> > > > >
>> > > >> > > > > For example:
>> > > >> > > > > def test_foo(self):
>> > > >> > > > > wtih self.assertRaises(AirflowException):
>> > > >> > > > > bar()
>> > > >> > > > >
>> > > >> > > > > Can be replaced by following code:
>> > > >> > > > >
>> > > >> > > > > from pytest import raises
>> > > >> > > > >
>> > > >> > > > > wtih raises(AirflowException):
>> > > >> > > > > bar()
>> > > >> > > > >
>> > > >> > > > > OR
>> > > >> > > > >
>> > > >> > > > > from parametrize import parametrize
>> > > >> > > > >
>> > > >> > > > > @parametrize.expand([
>> > > >> > > > > (1, 1, ),
>> > > >> > > > > (2, 2, ),
>> > > >> > > > > ])
>> > > >> > > > > def test_foo(self, param_a, param_b);
>> > > >> > > > > self.assertEqual(param_a, param_b)
>> > > >> > > > >
>> > > >> > > > > can be replaced by
>> > > >> > > > >
>> > > >> > > > > @pytest.mark.parametrize("param_a,param_b", [(1, 1), (2,
>> 2),])
>> > > >> > > > > def test_eval(param_a, param_b):
>> > > >> > > > > assert param_a == param_b
>> > > >> > > > >
>> > > >> > > > > OR
>> > > >> > > > >
>> > > >> > > > > @unittest.skip("demonstrating skipping")
>> > > >> > > > > def test_foo(self)
>> > > >> > > > >
>> > > >> > > > > can be replaced by
>> > > >> > > > >
>> > > >> > > > > @pytest.mark.skip(reason="demonstrating skipping"))
>> > > >> > > > > def test_foo(self)
>> > > >> > > > >
>> > > >> > > > > Which solution will be better for us?
>> > > >> > > > >
>> > > >> > > > > >
>> > > >> > > > > > On Wed, Jan 1, 2020 at 8:14 PM Kamil BreguĆa <
>> > > >> > > > kamil.bregula@polidea.com>
>> > > >> > > > > > wrote:
>> > > >> > > > > >
>> > > >> > > > > > > > 1) Do we want to use additional plugins?
>> > > >> > > > > > >
>> > > >> > > > > > > Yes. We should use the full-power of plugins now.
>> > > >> > > > > > >
>> > > >> > > > > > > > 2) Do we want to use custom markers?
>> > > >> > > > > > >
>> > > >> > > > > > > Reply in a separate thread.
>> > > >> > > > > > >
>> > > >> > > > > > > >3) Do we want to use new assert statement?
>> > > >> > > > > > >
>> > > >> > > > > > > Reply in a separate thread
>> > > >> > > > > > >
>> > > >> > > > > > > > 4) Do we want to remove the inheritance from
>> > > >> unittest.TestCase?
>> > > >> > > > > > >
>> > > >> > > > > > > Yes. After dropping support for Airflow 2.0, if
>> possible. I
>> > > >> would
>> > > >> > > > > prefer to
>> > > >> > > > > > > focus on working on new features for Airflow 2.0.
>> > > >> > > > > > >
>> > > >> > > > > > > > 5) Do we want to use class-less tests?
>> > > >> > > > > > >
>> > > >> > > > > > > No. Classes allow easy grouping of tests. Even if a
>> file with
>> > > >> one
>> > > >> > > > > class now
>> > > >> > > > > > > exists, a new one may appear in the future.
>> > > >> > > > > > >
>> > > >> > > > > > > > 6) Do we want to use pytest function instead of
>> current?
>> > > >> > > > > > >
>> > > >> > > > > > > I feel good about the current functions. However, this
>> is not
>> > > >> a
>> > > >> > > > serious
>> > > >> > > > > > > relationship and I can create a new friendship.
>> > > >> > > > > > >
>> > > >> > > > > > > > 7) Do we want to use monkeypatch fixture?
>> > > >> > > > > > >
>> > > >> > > > > > > No. I prefer unittest.mock
>> > > >> > > > > > >
>> > > >> > > > > > > > 8) Do we want to use the pytest fixtures?
>> > > >> > > > > > >
>> > > >> > > > > > > No. I prefer classic fixtures, if possible. Their
>> syntax is
>> > > >> much
>> > > >> > > > > simpler
>> > > >> > > > > > > and easier to understand.
>> > > >> > > > > > >
>> > > >> > > > > > > On Wed, Jan 1, 2020 at 8:10 PM Kamil BreguĆa <
>> > > >> > > > > kamil.bregula@polidea.com>
>> > > >> > > > > > > wrote:
>> > > >> > > > > > >
>> > > >> > > > > > > > Hello,
>> > > >> > > > > > > >
>> > > >> > > > > > > >
>> > > >> > > > > > > > We have recently migrated to pytest. Code written
>> > > >> according to
>> > > >> > > the
>> > > >> > > > > > > > standard library - unittest.TestCase is still
>> compatible
>> > > >> with
>> > > >> > > > pytest.
>> > > >> > > > > > > >
>> > > >> > > > > > > >
>> > > >> > > > > > > > The AIP-21 document did not discuss the migration of
>> > > >> current code
>> > > >> > > > to
>> > > >> > > > > > > > new features, but only discussed the change of runner
>> and
>> > > >> > > benefits
>> > > >> > > > of
>> > > >> > > > > > > > pytest over nosetets.
>> > > >> > > > > > > >
>> > > >> > > > > > > > Link:
>> > > >> > > > > > > >
>> > > >> > > > > > >
>> > > >> > > > >
>> > > >> > > >
>> > > >> > >
>> > > >>
>> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-27+Migrate+to+pytest
>> > > >> > > > > > > >
>> > > >> > > > > > > >
>> > > >> > > > > > > > As far as I appreciate the many advantages of using
>> this
>> > > >> tool,
>> > > >> > > but
>> > > >> > > > I
>> > > >> > > > > am
>> > > >> > > > > > > > not sure **whether, how or when we want to start
>> using some
>> > > >> > > > > features**. I
>> > > >> > > > > > > > prefer to keep the current project conventions in many
>> > > >> areas,
>> > > >> > > but I
>> > > >> > > > > still
>> > > >> > > > > > > > love pytest. I think we should establish general
>> > > >> conventions on
>> > > >> > > > > writing
>> > > >> > > > > > > > tests and recommended solutions to known problems. I
>> prefer
>> > > >> a
>> > > >> > > > > pragmatic
>> > > >> > > > > > > > approach, not just the use of features, because now
>> we can
>> > > >> use
>> > > >> > > it.
>> > > >> > > > > > > > Unfortunately, I do not see many benefits from new
>> features.
>> > > >> > > > > > > >
>> > > >> > > > > > > >
>> > > >> > > > > > > > I would not like the code to have many ways of
>> expressing
>> > > >> the
>> > > >> > > same
>> > > >> > > > > > > > solution. For the following reasons:
>> > > >> > > > > > > >
>> > > >> > > > > > > >
>> > > >> > > > > > > > * it can introduce a lack of understanding among new
>> > > >> contributors
>> > > >> > > > > > > >
>> > > >> > > > > > > > * facilitate the understanding and reading of code.
>> > > >> > > > > > > >
>> > > >> > > > > > > > * creates unnecessary discussion about the
>> preferences of
>> > > >> one way
>> > > >> > > > > over
>> > > >> > > > > > > the
>> > > >> > > > > > > > other. Not related to changes.
>> > > >> > > > > > > >
>> > > >> > > > > > > > * forces an understanding of the complex syntax of
>> some
>> > > >> > > solutions.
>> > > >> > > > > > > >
>> > > >> > > > > > > > * encourages people to rewrite the code, which can
>> generate
>> > > >> > > > > unnecessary
>> > > >> > > > > > > > work.
>> > > >> > > > > > > >
>> > > >> > > > > > > >
>> > > >> > > > > > > > To establish a full convention, I have prepared a few
>> > > >> questions:
>> > > >> > > > > > > >
>> > > >> > > > > > > > 1) Do we want to use additional plugins when there is
>> no
>> > > >> function
>> > > >> > > > in
>> > > >> > > > > the
>> > > >> > > > > > > > standard library e.g. flaky marker, forked marker?
>> This
>> > > >> is, in
>> > > >> > > my
>> > > >> > > > > > > opinion,
>> > > >> > > > > > > > one of the big advantages of migrating to pytest.
>> > > >> > > > > > > >
>> > > >> > > > > > > > 2) Do we want to use custom markers?
>> > > >> > > > > > > >
>> > > >> > > > > > > > The discussion takes place in a separate thread:
>> > > >> > > > > > > >
>> > > >> > > > > > > >
>> > > >> > > > > > > >
>> > > >> > > > > > >
>> > > >> > > > >
>> > > >> > > >
>> > > >> > >
>> > > >>
>> https://lists.apache.org/thread.html/4538437c96f599766005ba7829d0bee1511debb4f53599e0d300a56f%40%3Cdev.airflow.apache.org%3E
>> > > >> > > > > > > >
>> > > >> > > > > > > > 3) Do we want to use new assert statement?
>> > > >> > > > > > > >
>> > > >> > > > > > > >
>> > > >> > > > > > > >
>> > > >> > > > > > >
>> > > >> > > > >
>> > > >> > > >
>> > > >> > >
>> > > >>
>> https://lists.apache.org/thread.html/08b64d3b084c865399f98f6c6f56235ce5329e843d97938e1a8045a5%40%3Cdev.airflow.apache.org%3E
>> > > >> > > > > > > >
>> > > >> > > > > > > > Based on the discussion with devlist, we want to delay
>> > > >> migrations
>> > > >> > > > to
>> > > >> > > > > the
>> > > >> > > > > > > > new assert statement.
>> > > >> > > > > > > >
>> > > >> > > > > > > > 4) Do we want to remove inheritance from
>> unittest.TestCase?
>> > > >> > > > > > > >
>> > > >> > > > > > > > 5) Do we want to use class-less tests?
>> > > >> > > > > > > >
>> > > >> > > > > > > > 6) Do we want to use pytest function instead of
>> current?
>> > > >> > > > > > > >
>> > > >> > > > > > > > For example:
>> > > >> > > > > > > >
>> > > >> > > > > > > > Unittest.assertRaises vs pytest.raises
>> > > >> > > > > > > >
>> > > >> > > > > > > > parametrize vs pytest.mark.parametrize
>> > > >> > > > > > > >
>> > > >> > > > > > > > unittest.skip[If], vs pytest.mark.skip[If]
>> > > >> > > > > > > >
>> > > >> > > > > > > > 7) Do we want to use monkeypatch fixture?
>> > > >> > > > > > > >
>> > > >> > > > > > > > https://docs.pytest.org/en/latest/monkeypatch.html
>> > > >> > > > > > > >
>> > > >> > > > > > > > 8) Do we want to use the pytest fixtures?
>> > > >> > > > > > > >
>> > > >> > > > > > > > Do we want to migrate all code to pytest fixtures?
>> > > >> > > > > > > >
>> > > >> > > > > > > > We are currently using a different style of fixtures.
>> Do we
>> > > >> want
>> > > >> > > to
>> > > >> > > > > give
>> > > >> > > > > > > > it up?
>> > > >> > > > > > > >
>> > > >> > > > > > > >
>> > > >> > > > > > >
>> > > >> > > > >
>> > > >> > > >
>> > > >> > >
>> > > >>
>> https://docs.python.org/3/library/unittest.html#class-and-module-fixtures
>> > > >> > > > > > > >
>> > > >> > > > > > > >
>> > > >> > > > > > > > It is worth asking to think about whether we do not
>> want to
>> > > >> > > change
>> > > >> > > > > this
>> > > >> > > > > > > > convention in the future e.g. after dropping support
>> for
>> > > >> 1.10.X.
>> > > >> > > We
>> > > >> > > > > can
>> > > >> > > > > > > > also allow two conventions on a temporary basis, and
>> then
>> > > >> migrate
>> > > >> > > > to
>> > > >> > > > > one
>> > > >> > > > > > > at
>> > > >> > > > > > > > a later time. For example, we want to migrate to the
>> assert
>> > > >> > > > statement
>> > > >> > > > > > > after
>> > > >> > > > > > > > dropping support for 1.10
>> > > >> > > > > > > >
>> > > >> > > > > > > >
>> > > >> > > > > > > > I hope I found the main differences between the
>> current
>> > > >> > > convention
>> > > >> > > > > and
>> > > >> > > > > > > the
>> > > >> > > > > > > > new convention. However, if you missed something,
>> please
>> > > >> continue
>> > > >> > > > to
>> > > >> > > > > > > number.
>> > > >> > > > > > > >
>> > > >> > > > > > > >
>> > > >> > > > > > > > Best regards,
>> > > >> > > > > > > >
>> > > >> > > > > > > > Kamil
>> > > >> > > > > > > >
>> > > >> > > > > > > >
>> > > >> > > > > > > >
>> > > >> > > > > > > >
>> > > >> > > > > > >
>> > > >> > > > >
>> > > >> > > >
>> > > >> > >
>> > > >> > >
>> > > >> > > --
>> > > >> > >
>> > > >> > > Jarek Potiuk
>> > > >> > > Polidea <https://www.polidea.com/> | Principal Software
>> Engineer
>> > > >> > >
>> > > >> > > M: +48 660 796 129 <+48660796129>
>> > > >> > > [image: Polidea] <https://www.polidea.com/>
>> > > >> > >
>> > > >> >
>> > > >> >
>> > > >> > --
>> > > >> >
>> > > >> > Tomasz Urbaszek
>> > > >> > Polidea <https://www.polidea.com/> | Software Engineer
>> > > >> >
>> > > >> > M: +48 505 628 493 <+48505628493>
>> > > >> > E: tomasz.urbaszek@polidea.com <to...@polidea.com>
>> > > >> >
>> > > >> > Unique Tech
>> > > >> > Check out our projects! <https://www.polidea.com/our-work>
>> > > >>
>> > > >
>> > > >
>> > > > --
>> > > >
>> > > > Jarek Potiuk
>> > > > Polidea <https://www.polidea.com/> | Principal Software Engineer
>> > > >
>> > > > M: +48 660 796 129 <+48660796129>
>> > > > [image: Polidea] <https://www.polidea.com/>
>> > > >
>> > > >
>> > >
>> > > --
>> > >
>> > > Jarek Potiuk
>> > > Polidea <https://www.polidea.com/> | Principal Software Engineer
>> > >
>> > > M: +48 660 796 129 <+48660796129>
>> > > [image: Polidea] <https://www.polidea.com/>
>> >
>>
>>
Re: Test style convention after migration to pytest
Posted by Tomasz Urbaszek <tu...@apache.org>.
Hello all!
The PR for changing self.assert to plain assert is here:
https://github.com/apache/airflow/pull/12951
All tests seem to be passing (except on heisentest) so I'm going to merge
on Monday if there are no objections.
Cheers,
Tomek
On Thu, Oct 22, 2020 at 1:04 PM Ash Berlin-Taylor <as...@apache.org> wrote:
> 2. We already do don't we for backend and integrations etc?
> 3. I am strongly in favour of -- using assert with pytest gives much
> nicer error messages than unittest's own assertions;
> 4. if we do 3, then I'm also in favour of doing 4 as we don't need it
> anymore, and we can then use more plugins/features of pytest (parameterized
> behaves better I think)
> 5. No view for or against
> 6. depends on what we do for 4
> 7. unittest.mock can be confusing to use, but I'm not sure pytests
> monkeypatch fixture is nicer. No opinoin on this one
> 8. I like them, but the automatic dependency injection can be confusing
> until you are familar with Pytest. An example of them for anyone unaware:
> https://github.com/apache/airflow/pull/11694 (look at the example test at
> the end of the PR description)
>
> I don't think we need to (yet) do any bulk changes to the test, but if we
> agree that using more of pytest is the way to go then we can write new
> tests using assert style, and once we've got a good chunk of the way we can
> bulk switch the rest.
>
> -ash
>
> On Oct 22 2020, at 11:45 am, Tomasz Urbaszek <tu...@apache.org> wrote:
>
> Hi all,
>
> I would like to resurrect this discussion as another one was started in
> https://github.com/apache/airflow/issues/11657#issuecomment-713834942
>
> Main questions were:
>
> > 1) Do we want to use additional plugins when there is no function in
> > the standard library e.g. flaky marker, forked marker?
> > 2) Do we want to use custom markers?
> > 3) Do we want to use new assert statement?
> > 4) Do we want to remove inheritance from unittest.TestCase?
> > 5) Do we want to use class-less tests?
> > 6) Do we want to use pytest function instead of current?
> > 7) Do we want to use monkeypatch fixture?
> > 8) Do we want to use the pytest fixtures?
>
> Probably any changes we decide on should be enforced after 2.0.
>
> Cheers,
> Tomek
>
> On 2020/01/18 01:08:29, Kamil BreguĆa <ka...@polidea.com> wrote:
> > Hello,
> >
> > Sorry for the late reply. Recently, I have been very busy with the
> > client project and work on the AIP-21.
> >
> > Let's start. I split this message into several sections for easier
> reading.
> >
> > **Establishment of a convention**
> > I will prepare a summary. It is available here:
> >
> >
> https://docs.google.com/spreadsheets/d/10qToeTFMxjShBX3xcK6IWr9Sp8Pw7qk35fQTtzpZYYs/edit?usp=sharing
> >
> > The mailing list does not allow rich content, so I copy the verdicts.
> >
> > 1) Do we want to use additional plugins when there is no function in
> > the standard library e.g. flaky marker, forked marker?YES
> > 2) Do we want to use custom markers? YES
> > 3) Do we want to use new assert statement? YES
> > 4) Do we want to remove inheritance from unittest.TestCase? YES
> > 5) Do we want to use class-less tests? NO
> > 6) Do we want to use pytest function instead of current? No verdict
> > 7) Do we want to use monkeypatch fixture? NO
> > 8) Do we want to use the pytest fixtures? YES
> >
> > We don't have a verdict in question 6, because we have no answer from
> > Tomek and Jarek. Can you answer this question? A detailed explanation
> > is available.
> >
> https://lists.apache.org/thread.html/7ae7ba0e72d3f0d12f8398a85980c5064b58574caa727c6a974fc628%40%3Cdev.airflow.apache.org%3E
> > Should we use the pytest functions or use the standard functions?
> >
> > **Using the new convention in the current code**
> > tl;dr; Two conventions will be accepted simultaneously.
> > Long story: As for the time of using the new convention, I'm the only
> > one who is worried about whether to allow it now. Other people did not
> > share my fears. So I think we should allow the use of the new
> > conventions now. We want to automate the introduction of the new
> > convention in the future, so I think that the application of the two
> > conventions should be allowed temporarily. However, I will describe
> > the new conventions, but we do not need to strictly require them until
> > the remaining code is migrated.
> >
> > **Full migration to the new convention**
> > tl;dr; We'll do it later.
> > Long story: Should we migrate the remaining code now? Tomek prefers
> > to wait for the end of work on AIP-21. Kaxil and I also prefer to wait
> > for AIP-21 or AIrflow 2.0. Jarek prefers to wait until we finish all
> > other works that improve the development environment e.g. pylint. So
> > we everybody prefers to wait. I think we can go back to the discussion
> > when the pylint is completed or we finish AIP-21. It depends on which
> > will be earlier.
> >
> > Best regards,
> > Kamil
> >
> > On Fri, Jan 17, 2020 at 3:07 PM Jarek Potiuk <Ja...@polidea.com>
> wrote:
> > >
> > > Should we resume this :)?
> > >
> > > On Mon, Jan 6, 2020 at 3:42 PM Jarek Potiuk <Ja...@polidea.com>
> > > wrote:
> > >
> > > > Good idea!
> > > >
> > > > On Mon, Jan 6, 2020 at 3:12 PM Kamil BreguĆa <
> kamil.bregula@polidea.com>
> > > > wrote:
> > > >
> > > >> Hello,
> > > >>
> > > >> Now is the holiday period. Some people have not started working yet.
> > > >> Others are busy with New Year activities. What do you think about
> > > >> Friday?
> > > >>
> > > >> Best regards,
> > > >> Kamil
> > > >>
> > > >> On Mon, Jan 6, 2020 at 2:48 PM Tomasz Urbaszek
> > > >> <to...@polidea.com> wrote:
> > > >> >
> > > >> > When should we assume that we've reached a consensus?
> > > >> >
> > > >> > T.
> > > >> >
> > > >> > On Thu, Jan 2, 2020 at 12:52 AM Jarek Potiuk <
> Jarek.Potiuk@polidea.com>
> > > >> > wrote:
> > > >> >
> > > >> > > > 1) Do we want to use additional plugins? *Yes*
> > > >> > >
> > > >> > > 2) Do we want to use custom markers? *Yes. They will help with
> > > >> optimising
> > > >> > > > our test execution.*
> > > >> > >
> > > >> > > 3) Do we want to use new assert statement? *Yes*
> > > >> > > > 4) Do we want to remove the inheritance from
> unittest.TestCase? *No
> > > >> > > > opinion about it. I am ok with both.*
> > > >> > > > 5) Do we want to use class-less tests? *No.*
> > > >> > > > 6) Do we want to use pytest function instead of current? *I
> don't
> > > >> > > > understand. Can you explain please?*
> > > >> > > > 7) Do we want to use monkeypatch fixture? *No. Mock is
> better.*
> > > >> > > > 8) Do we want to use the pytest fixtures? *Yes.*
> > > >> > > >
> > > >> > >
> > > >> > > Great that we have this discussion now. I also think we should
> just
> > > >> agree
> > > >> > > it now and not introduce it "globally".
> > > >> > > Once we do it, we should simply add it together new features we
> > > >> implement.
> > > >> > > We have still pylint to finish as a "non-functional global
> change"
> > > >> and we
> > > >> > > should not add new one. It's good to continually improve but one
> > > >> thing at a
> > > >> > > time.
> > > >> > >
> > > >> > > BTW Pylint goes well we are down to 243 non-pylint files from
> 991
> > > >> since
> > > >> > > May.
> > > >> > >
> > > >> > > J.
> > > >> > >
> > > >> > > On Thu, Jan 2, 2020 at 12:40 AM Kaxil Naik <kaxilnaik@gmail.com
> >
> > > >> wrote:
> > > >> > >
> > > >> > > > This is a tough one. Both arguments are reasonable.
> > > >> > > >
> > > >> > > > I agree at some point we should convert all to use assert.
> But at
> > > >> the
> > > >> > > same
> > > >> > > > time, we should also focus on adding *more user-facing
> features *and
> > > >> > > spend
> > > >> > > > less time on more refactor or similar changes.
> > > >> > > >
> > > >> > > > So based on that, this might be a low priority. We also need
> to
> > > >> still
> > > >> > > > complete AIP-21
> > > >> > > > <
> > > >> > > >
> > > >> > >
> > > >>
> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-21%3A+Changes+in+import+paths
> > > >> > > > >
> > > >> > > > which is very critical for 2.0.
> > > >> > > >
> > > >> > > > 1) Do we want to use additional plugins?
> > > >> > > >
> > > >> > > >
> > > >> > > > Yes.
> > > >> > > >
> > > >> > > > 2) Do we want to use custom markers?
> > > >> > > >
> > > >> > > >
> > > >> > > > Not sure yet. Low Priority for me.
> > > >> > > >
> > > >> > > > 3) Do we want to use new assert statement?
> > > >> > > >
> > > >> > > >
> > > >> > > > I think new PRs can contain it, shouldn't be a problem as
> long as
> > > >> it is
> > > >> > > > documented to avoid confusion.
> > > >> > > >
> > > >> > > > 4) Do we want to remove the inheritance from
> unittest.TestCase?
> > > >> > > >
> > > >> > > >
> > > >> > > > Yes, this is, however, going to change how people write
> tests. So
> > > >> someone
> > > >> > > > has to own it as it can become painful with PRs getting merged
> > > >> > > > continuously.
> > > >> > > >
> > > >> > > > 5) Do we want to use class-less tests?
> > > >> > > >
> > > >> > > >
> > > >> > > > No.
> > > >> > > >
> > > >> > > > 6) Do we want to use pytest function instead of current?
> > > >> > > >
> > > >> > > >
> > > >> > > > Yes
> > > >> > > >
> > > >> > > > 7) Do we want to use monkeypatch fixture?
> > > >> > > >
> > > >> > > >
> > > >> > > > I also prefer unittest.mock but open to suggestions.
> > > >> > > >
> > > >>
> https://github.com/pytest-dev/pytest/issues/4576#issuecomment-449864333
> > > >> > > > has
> > > >> > > > some good comparison on it
> > > >> > > >
> > > >> > > > 8) Do we want to use the pytest fixtures?
> > > >> > > >
> > > >> > > >
> > > >> > > > Yes
> > > >> > > >
> > > >> > > > On Wed, Jan 1, 2020 at 8:07 PM Kamil BreguĆa <
> > > >> kamil.bregula@polidea.com>
> > > >> > > > wrote:
> > > >> > > >
> > > >> > > > > @unittest.skip("demonstrating skipping")
> > > >> > > > >
> > > >> > > > > On Wed, Jan 1, 2020 at 8:37 PM Tomasz Urbaszek <
> > > >> turbaszek@apache.org>
> > > >> > > > > wrote:
> > > >> > > > > >
> > > >> > > > > >
> > > >> > > > > > > 6) Do we want to use pytest function instead of current?
> > > >> > > > > >
> > > >> > > > > > I do not understand this point, can you explain?
> > > >> > > > > >
> > > >> > > > >
> > > >> > > > > Pytest Introduces solutions that replace solutions that are
> now
> > > >> used
> > > >> > > > >
> > > >> > > > > For example:
> > > >> > > > > def test_foo(self):
> > > >> > > > > wtih self.assertRaises(AirflowException):
> > > >> > > > > bar()
> > > >> > > > >
> > > >> > > > > Can be replaced by following code:
> > > >> > > > >
> > > >> > > > > from pytest import raises
> > > >> > > > >
> > > >> > > > > wtih raises(AirflowException):
> > > >> > > > > bar()
> > > >> > > > >
> > > >> > > > > OR
> > > >> > > > >
> > > >> > > > > from parametrize import parametrize
> > > >> > > > >
> > > >> > > > > @parametrize.expand([
> > > >> > > > > (1, 1, ),
> > > >> > > > > (2, 2, ),
> > > >> > > > > ])
> > > >> > > > > def test_foo(self, param_a, param_b);
> > > >> > > > > self.assertEqual(param_a, param_b)
> > > >> > > > >
> > > >> > > > > can be replaced by
> > > >> > > > >
> > > >> > > > > @pytest.mark.parametrize("param_a,param_b", [(1, 1), (2,
> 2),])
> > > >> > > > > def test_eval(param_a, param_b):
> > > >> > > > > assert param_a == param_b
> > > >> > > > >
> > > >> > > > > OR
> > > >> > > > >
> > > >> > > > > @unittest.skip("demonstrating skipping")
> > > >> > > > > def test_foo(self)
> > > >> > > > >
> > > >> > > > > can be replaced by
> > > >> > > > >
> > > >> > > > > @pytest.mark.skip(reason="demonstrating skipping"))
> > > >> > > > > def test_foo(self)
> > > >> > > > >
> > > >> > > > > Which solution will be better for us?
> > > >> > > > >
> > > >> > > > > >
> > > >> > > > > > On Wed, Jan 1, 2020 at 8:14 PM Kamil BreguĆa <
> > > >> > > > kamil.bregula@polidea.com>
> > > >> > > > > > wrote:
> > > >> > > > > >
> > > >> > > > > > > > 1) Do we want to use additional plugins?
> > > >> > > > > > >
> > > >> > > > > > > Yes. We should use the full-power of plugins now.
> > > >> > > > > > >
> > > >> > > > > > > > 2) Do we want to use custom markers?
> > > >> > > > > > >
> > > >> > > > > > > Reply in a separate thread.
> > > >> > > > > > >
> > > >> > > > > > > >3) Do we want to use new assert statement?
> > > >> > > > > > >
> > > >> > > > > > > Reply in a separate thread
> > > >> > > > > > >
> > > >> > > > > > > > 4) Do we want to remove the inheritance from
> > > >> unittest.TestCase?
> > > >> > > > > > >
> > > >> > > > > > > Yes. After dropping support for Airflow 2.0, if
> possible. I
> > > >> would
> > > >> > > > > prefer to
> > > >> > > > > > > focus on working on new features for Airflow 2.0.
> > > >> > > > > > >
> > > >> > > > > > > > 5) Do we want to use class-less tests?
> > > >> > > > > > >
> > > >> > > > > > > No. Classes allow easy grouping of tests. Even if a
> file with
> > > >> one
> > > >> > > > > class now
> > > >> > > > > > > exists, a new one may appear in the future.
> > > >> > > > > > >
> > > >> > > > > > > > 6) Do we want to use pytest function instead of
> current?
> > > >> > > > > > >
> > > >> > > > > > > I feel good about the current functions. However, this
> is not
> > > >> a
> > > >> > > > serious
> > > >> > > > > > > relationship and I can create a new friendship.
> > > >> > > > > > >
> > > >> > > > > > > > 7) Do we want to use monkeypatch fixture?
> > > >> > > > > > >
> > > >> > > > > > > No. I prefer unittest.mock
> > > >> > > > > > >
> > > >> > > > > > > > 8) Do we want to use the pytest fixtures?
> > > >> > > > > > >
> > > >> > > > > > > No. I prefer classic fixtures, if possible. Their
> syntax is
> > > >> much
> > > >> > > > > simpler
> > > >> > > > > > > and easier to understand.
> > > >> > > > > > >
> > > >> > > > > > > On Wed, Jan 1, 2020 at 8:10 PM Kamil BreguĆa <
> > > >> > > > > kamil.bregula@polidea.com>
> > > >> > > > > > > wrote:
> > > >> > > > > > >
> > > >> > > > > > > > Hello,
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > We have recently migrated to pytest. Code written
> > > >> according to
> > > >> > > the
> > > >> > > > > > > > standard library - unittest.TestCase is still
> compatible
> > > >> with
> > > >> > > > pytest.
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > The AIP-21 document did not discuss the migration of
> > > >> current code
> > > >> > > > to
> > > >> > > > > > > > new features, but only discussed the change of runner
> and
> > > >> > > benefits
> > > >> > > > of
> > > >> > > > > > > > pytest over nosetets.
> > > >> > > > > > > >
> > > >> > > > > > > > Link:
> > > >> > > > > > > >
> > > >> > > > > > >
> > > >> > > > >
> > > >> > > >
> > > >> > >
> > > >>
> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-27+Migrate+to+pytest
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > As far as I appreciate the many advantages of using
> this
> > > >> tool,
> > > >> > > but
> > > >> > > > I
> > > >> > > > > am
> > > >> > > > > > > > not sure **whether, how or when we want to start
> using some
> > > >> > > > > features**. I
> > > >> > > > > > > > prefer to keep the current project conventions in many
> > > >> areas,
> > > >> > > but I
> > > >> > > > > still
> > > >> > > > > > > > love pytest. I think we should establish general
> > > >> conventions on
> > > >> > > > > writing
> > > >> > > > > > > > tests and recommended solutions to known problems. I
> prefer
> > > >> a
> > > >> > > > > pragmatic
> > > >> > > > > > > > approach, not just the use of features, because now
> we can
> > > >> use
> > > >> > > it.
> > > >> > > > > > > > Unfortunately, I do not see many benefits from new
> features.
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > I would not like the code to have many ways of
> expressing
> > > >> the
> > > >> > > same
> > > >> > > > > > > > solution. For the following reasons:
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > * it can introduce a lack of understanding among new
> > > >> contributors
> > > >> > > > > > > >
> > > >> > > > > > > > * facilitate the understanding and reading of code.
> > > >> > > > > > > >
> > > >> > > > > > > > * creates unnecessary discussion about the
> preferences of
> > > >> one way
> > > >> > > > > over
> > > >> > > > > > > the
> > > >> > > > > > > > other. Not related to changes.
> > > >> > > > > > > >
> > > >> > > > > > > > * forces an understanding of the complex syntax of
> some
> > > >> > > solutions.
> > > >> > > > > > > >
> > > >> > > > > > > > * encourages people to rewrite the code, which can
> generate
> > > >> > > > > unnecessary
> > > >> > > > > > > > work.
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > To establish a full convention, I have prepared a few
> > > >> questions:
> > > >> > > > > > > >
> > > >> > > > > > > > 1) Do we want to use additional plugins when there is
> no
> > > >> function
> > > >> > > > in
> > > >> > > > > the
> > > >> > > > > > > > standard library e.g. flaky marker, forked marker?
> This
> > > >> is, in
> > > >> > > my
> > > >> > > > > > > opinion,
> > > >> > > > > > > > one of the big advantages of migrating to pytest.
> > > >> > > > > > > >
> > > >> > > > > > > > 2) Do we want to use custom markers?
> > > >> > > > > > > >
> > > >> > > > > > > > The discussion takes place in a separate thread:
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > >
> > > >> > > > >
> > > >> > > >
> > > >> > >
> > > >>
> https://lists.apache.org/thread.html/4538437c96f599766005ba7829d0bee1511debb4f53599e0d300a56f%40%3Cdev.airflow.apache.org%3E
> > > >> > > > > > > >
> > > >> > > > > > > > 3) Do we want to use new assert statement?
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > >
> > > >> > > > >
> > > >> > > >
> > > >> > >
> > > >>
> https://lists.apache.org/thread.html/08b64d3b084c865399f98f6c6f56235ce5329e843d97938e1a8045a5%40%3Cdev.airflow.apache.org%3E
> > > >> > > > > > > >
> > > >> > > > > > > > Based on the discussion with devlist, we want to delay
> > > >> migrations
> > > >> > > > to
> > > >> > > > > the
> > > >> > > > > > > > new assert statement.
> > > >> > > > > > > >
> > > >> > > > > > > > 4) Do we want to remove inheritance from
> unittest.TestCase?
> > > >> > > > > > > >
> > > >> > > > > > > > 5) Do we want to use class-less tests?
> > > >> > > > > > > >
> > > >> > > > > > > > 6) Do we want to use pytest function instead of
> current?
> > > >> > > > > > > >
> > > >> > > > > > > > For example:
> > > >> > > > > > > >
> > > >> > > > > > > > Unittest.assertRaises vs pytest.raises
> > > >> > > > > > > >
> > > >> > > > > > > > parametrize vs pytest.mark.parametrize
> > > >> > > > > > > >
> > > >> > > > > > > > unittest.skip[If], vs pytest.mark.skip[If]
> > > >> > > > > > > >
> > > >> > > > > > > > 7) Do we want to use monkeypatch fixture?
> > > >> > > > > > > >
> > > >> > > > > > > > https://docs.pytest.org/en/latest/monkeypatch.html
> > > >> > > > > > > >
> > > >> > > > > > > > 8) Do we want to use the pytest fixtures?
> > > >> > > > > > > >
> > > >> > > > > > > > Do we want to migrate all code to pytest fixtures?
> > > >> > > > > > > >
> > > >> > > > > > > > We are currently using a different style of fixtures.
> Do we
> > > >> want
> > > >> > > to
> > > >> > > > > give
> > > >> > > > > > > > it up?
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > >
> > > >> > > > >
> > > >> > > >
> > > >> > >
> > > >>
> https://docs.python.org/3/library/unittest.html#class-and-module-fixtures
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > It is worth asking to think about whether we do not
> want to
> > > >> > > change
> > > >> > > > > this
> > > >> > > > > > > > convention in the future e.g. after dropping support
> for
> > > >> 1.10.X.
> > > >> > > We
> > > >> > > > > can
> > > >> > > > > > > > also allow two conventions on a temporary basis, and
> then
> > > >> migrate
> > > >> > > > to
> > > >> > > > > one
> > > >> > > > > > > at
> > > >> > > > > > > > a later time. For example, we want to migrate to the
> assert
> > > >> > > > statement
> > > >> > > > > > > after
> > > >> > > > > > > > dropping support for 1.10
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > I hope I found the main differences between the
> current
> > > >> > > convention
> > > >> > > > > and
> > > >> > > > > > > the
> > > >> > > > > > > > new convention. However, if you missed something,
> please
> > > >> continue
> > > >> > > > to
> > > >> > > > > > > number.
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > Best regards,
> > > >> > > > > > > >
> > > >> > > > > > > > Kamil
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > >
> > > >> > > > >
> > > >> > > >
> > > >> > >
> > > >> > >
> > > >> > > --
> > > >> > >
> > > >> > > Jarek Potiuk
> > > >> > > Polidea <https://www.polidea.com/> | Principal Software
> Engineer
> > > >> > >
> > > >> > > M: +48 660 796 129 <+48660796129>
> > > >> > > [image: Polidea] <https://www.polidea.com/>
> > > >> > >
> > > >> >
> > > >> >
> > > >> > --
> > > >> >
> > > >> > Tomasz Urbaszek
> > > >> > Polidea <https://www.polidea.com/> | Software Engineer
> > > >> >
> > > >> > M: +48 505 628 493 <+48505628493>
> > > >> > E: tomasz.urbaszek@polidea.com <to...@polidea.com>
> > > >> >
> > > >> > Unique Tech
> > > >> > Check out our projects! <https://www.polidea.com/our-work>
> > > >>
> > > >
> > > >
> > > > --
> > > >
> > > > Jarek Potiuk
> > > > Polidea <https://www.polidea.com/> | Principal Software Engineer
> > > >
> > > > M: +48 660 796 129 <+48660796129>
> > > > [image: Polidea] <https://www.polidea.com/>
> > > >
> > > >
> > >
> > > --
> > >
> > > Jarek Potiuk
> > > Polidea <https://www.polidea.com/> | Principal Software Engineer
> > >
> > > M: +48 660 796 129 <+48660796129>
> > > [image: Polidea] <https://www.polidea.com/>
> >
>
>
Re: Test style convention after migration to pytest
Posted by Ash Berlin-Taylor <as...@apache.org>.
2. We already do don't we for backend and integrations etc?
3. I am strongly in favour of -- using assert with pytest gives much nicer error messages than unittest's own assertions;
4. if we do 3, then I'm also in favour of doing 4 as we don't need it anymore, and we can then use more plugins/features of pytest (parameterized behaves better I think)
5. No view for or against
6. depends on what we do for 4
7. unittest.mock can be confusing to use, but I'm not sure pytests monkeypatch fixture is nicer. No opinoin on this one
8. I like them, but the automatic dependency injection can be confusing until you are familar with Pytest. An example of them for anyone unaware: https://github.com/apache/airflow/pull/11694 (look at the example test at the end of the PR description)
I don't think we need to (yet) do any bulk changes to the test, but if we agree that using more of pytest is the way to go then we can write new tests using assert style, and once we've got a good chunk of the way we can bulk switch the rest.
-ash
On Oct 22 2020, at 11:45 am, Tomasz Urbaszek <tu...@apache.org> wrote:
> Hi all,
>
> I would like to resurrect this discussion as another one was started in https://github.com/apache/airflow/issues/11657#issuecomment-713834942
> Main questions were:
> > 1) Do we want to use additional plugins when there is no function in
> > the standard library e.g. flaky marker, forked marker?
> > 2) Do we want to use custom markers?
> > 3) Do we want to use new assert statement?
> > 4) Do we want to remove inheritance from unittest.TestCase?
> > 5) Do we want to use class-less tests?
> > 6) Do we want to use pytest function instead of current?
> > 7) Do we want to use monkeypatch fixture?
> > 8) Do we want to use the pytest fixtures?
>
> Probably any changes we decide on should be enforced after 2.0.
> Cheers,
> Tomek
>
> On 2020/01/18 01:08:29, Kamil BreguĆa <ka...@polidea.com> wrote:
> > Hello,
> >
> > Sorry for the late reply. Recently, I have been very busy with the
> > client project and work on the AIP-21.
> >
> > Let's start. I split this message into several sections for easier reading.
> >
> > **Establishment of a convention**
> > I will prepare a summary. It is available here:
> >
> > https://docs.google.com/spreadsheets/d/10qToeTFMxjShBX3xcK6IWr9Sp8Pw7qk35fQTtzpZYYs/edit?usp=sharing
> >
> > The mailing list does not allow rich content, so I copy the verdicts.
> >
> > 1) Do we want to use additional plugins when there is no function in
> > the standard library e.g. flaky marker, forked marker?YES
> > 2) Do we want to use custom markers? YES
> > 3) Do we want to use new assert statement? YES
> > 4) Do we want to remove inheritance from unittest.TestCase? YES
> > 5) Do we want to use class-less tests? NO
> > 6) Do we want to use pytest function instead of current? No verdict
> > 7) Do we want to use monkeypatch fixture? NO
> > 8) Do we want to use the pytest fixtures? YES
> >
> > We don't have a verdict in question 6, because we have no answer from
> > Tomek and Jarek. Can you answer this question? A detailed explanation
> > is available.
> > https://lists.apache.org/thread.html/7ae7ba0e72d3f0d12f8398a85980c5064b58574caa727c6a974fc628%40%3Cdev.airflow.apache.org%3E
> > Should we use the pytest functions or use the standard functions?
> >
> > **Using the new convention in the current code**
> > tl;dr; Two conventions will be accepted simultaneously.
> > Long story: As for the time of using the new convention, I'm the only
> > one who is worried about whether to allow it now. Other people did not
> > share my fears. So I think we should allow the use of the new
> > conventions now. We want to automate the introduction of the new
> > convention in the future, so I think that the application of the two
> > conventions should be allowed temporarily. However, I will describe
> > the new conventions, but we do not need to strictly require them until
> > the remaining code is migrated.
> >
> > **Full migration to the new convention**
> > tl;dr; We'll do it later.
> > Long story: Should we migrate the remaining code now? Tomek prefers
> > to wait for the end of work on AIP-21. Kaxil and I also prefer to wait
> > for AIP-21 or AIrflow 2.0. Jarek prefers to wait until we finish all
> > other works that improve the development environment e.g. pylint. So
> > we everybody prefers to wait. I think we can go back to the discussion
> > when the pylint is completed or we finish AIP-21. It depends on which
> > will be earlier.
> >
> > Best regards,
> > Kamil
> >
> > On Fri, Jan 17, 2020 at 3:07 PM Jarek Potiuk <Ja...@polidea.com> wrote:
> > >
> > > Should we resume this :)?
> > >
> > > On Mon, Jan 6, 2020 at 3:42 PM Jarek Potiuk <Ja...@polidea.com>
> > > wrote:
> > >
> > > > Good idea!
> > > >
> > > > On Mon, Jan 6, 2020 at 3:12 PM Kamil BreguĆa <ka...@polidea.com>
> > > > wrote:
> > > >
> > > >> Hello,
> > > >>
> > > >> Now is the holiday period. Some people have not started working yet.
> > > >> Others are busy with New Year activities. What do you think about
> > > >> Friday?
> > > >>
> > > >> Best regards,
> > > >> Kamil
> > > >>
> > > >> On Mon, Jan 6, 2020 at 2:48 PM Tomasz Urbaszek
> > > >> <to...@polidea.com> wrote:
> > > >> >
> > > >> > When should we assume that we've reached a consensus?
> > > >> >
> > > >> > T.
> > > >> >
> > > >> > On Thu, Jan 2, 2020 at 12:52 AM Jarek Potiuk <Ja...@polidea.com>
> > > >> > wrote:
> > > >> >
> > > >> > > > 1) Do we want to use additional plugins? *Yes*
> > > >> > >
> > > >> > > 2) Do we want to use custom markers? *Yes. They will help with
> > > >> optimising
> > > >> > > > our test execution.*
> > > >> > >
> > > >> > > 3) Do we want to use new assert statement? *Yes*
> > > >> > > > 4) Do we want to remove the inheritance from unittest.TestCase? *No
> > > >> > > > opinion about it. I am ok with both.*
> > > >> > > > 5) Do we want to use class-less tests? *No.*
> > > >> > > > 6) Do we want to use pytest function instead of current? *I don't
> > > >> > > > understand. Can you explain please?*
> > > >> > > > 7) Do we want to use monkeypatch fixture? *No. Mock is better.*
> > > >> > > > 8) Do we want to use the pytest fixtures? *Yes.*
> > > >> > > >
> > > >> > >
> > > >> > > Great that we have this discussion now. I also think we should just
> > > >> agree
> > > >> > > it now and not introduce it "globally".
> > > >> > > Once we do it, we should simply add it together new features we
> > > >> implement.
> > > >> > > We have still pylint to finish as a "non-functional global change"
> > > >> and we
> > > >> > > should not add new one. It's good to continually improve but one
> > > >> thing at a
> > > >> > > time.
> > > >> > >
> > > >> > > BTW Pylint goes well we are down to 243 non-pylint files from 991
> > > >> since
> > > >> > > May.
> > > >> > >
> > > >> > > J.
> > > >> > >
> > > >> > > On Thu, Jan 2, 2020 at 12:40 AM Kaxil Naik <ka...@gmail.com>
> > > >> wrote:
> > > >> > >
> > > >> > > > This is a tough one. Both arguments are reasonable.
> > > >> > > >
> > > >> > > > I agree at some point we should convert all to use assert. But at
> > > >> the
> > > >> > > same
> > > >> > > > time, we should also focus on adding *more user-facing features *and
> > > >> > > spend
> > > >> > > > less time on more refactor or similar changes.
> > > >> > > >
> > > >> > > > So based on that, this might be a low priority. We also need to
> > > >> still
> > > >> > > > complete AIP-21
> > > >> > > > <
> > > >> > > >
> > > >> > >
> > > >> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-21%3A+Changes+in+import+paths
> > > >> > > > >
> > > >> > > > which is very critical for 2.0.
> > > >> > > >
> > > >> > > > 1) Do we want to use additional plugins?
> > > >> > > >
> > > >> > > >
> > > >> > > > Yes.
> > > >> > > >
> > > >> > > > 2) Do we want to use custom markers?
> > > >> > > >
> > > >> > > >
> > > >> > > > Not sure yet. Low Priority for me.
> > > >> > > >
> > > >> > > > 3) Do we want to use new assert statement?
> > > >> > > >
> > > >> > > >
> > > >> > > > I think new PRs can contain it, shouldn't be a problem as long as
> > > >> it is
> > > >> > > > documented to avoid confusion.
> > > >> > > >
> > > >> > > > 4) Do we want to remove the inheritance from unittest.TestCase?
> > > >> > > >
> > > >> > > >
> > > >> > > > Yes, this is, however, going to change how people write tests. So
> > > >> someone
> > > >> > > > has to own it as it can become painful with PRs getting merged
> > > >> > > > continuously.
> > > >> > > >
> > > >> > > > 5) Do we want to use class-less tests?
> > > >> > > >
> > > >> > > >
> > > >> > > > No.
> > > >> > > >
> > > >> > > > 6) Do we want to use pytest function instead of current?
> > > >> > > >
> > > >> > > >
> > > >> > > > Yes
> > > >> > > >
> > > >> > > > 7) Do we want to use monkeypatch fixture?
> > > >> > > >
> > > >> > > >
> > > >> > > > I also prefer unittest.mock but open to suggestions.
> > > >> > > >
> > > >> https://github.com/pytest-dev/pytest/issues/4576#issuecomment-449864333
> > > >> > > > has
> > > >> > > > some good comparison on it
> > > >> > > >
> > > >> > > > 8) Do we want to use the pytest fixtures?
> > > >> > > >
> > > >> > > >
> > > >> > > > Yes
> > > >> > > >
> > > >> > > > On Wed, Jan 1, 2020 at 8:07 PM Kamil BreguĆa <
> > > >> kamil.bregula@polidea.com>
> > > >> > > > wrote:
> > > >> > > >
> > > >> > > > > @unittest.skip("demonstrating skipping")
> > > >> > > > >
> > > >> > > > > On Wed, Jan 1, 2020 at 8:37 PM Tomasz Urbaszek <
> > > >> turbaszek@apache.org>
> > > >> > > > > wrote:
> > > >> > > > > >
> > > >> > > > > >
> > > >> > > > > > > 6) Do we want to use pytest function instead of current?
> > > >> > > > > >
> > > >> > > > > > I do not understand this point, can you explain?
> > > >> > > > > >
> > > >> > > > >
> > > >> > > > > Pytest Introduces solutions that replace solutions that are now
> > > >> used
> > > >> > > > >
> > > >> > > > > For example:
> > > >> > > > > def test_foo(self):
> > > >> > > > > wtih self.assertRaises(AirflowException):
> > > >> > > > > bar()
> > > >> > > > >
> > > >> > > > > Can be replaced by following code:
> > > >> > > > >
> > > >> > > > > from pytest import raises
> > > >> > > > >
> > > >> > > > > wtih raises(AirflowException):
> > > >> > > > > bar()
> > > >> > > > >
> > > >> > > > > OR
> > > >> > > > >
> > > >> > > > > from parametrize import parametrize
> > > >> > > > >
> > > >> > > > > @parametrize.expand([
> > > >> > > > > (1, 1, ),
> > > >> > > > > (2, 2, ),
> > > >> > > > > ])
> > > >> > > > > def test_foo(self, param_a, param_b);
> > > >> > > > > self.assertEqual(param_a, param_b)
> > > >> > > > >
> > > >> > > > > can be replaced by
> > > >> > > > >
> > > >> > > > > @pytest.mark.parametrize("param_a,param_b", [(1, 1), (2, 2),])
> > > >> > > > > def test_eval(param_a, param_b):
> > > >> > > > > assert param_a == param_b
> > > >> > > > >
> > > >> > > > > OR
> > > >> > > > >
> > > >> > > > > @unittest.skip("demonstrating skipping")
> > > >> > > > > def test_foo(self)
> > > >> > > > >
> > > >> > > > > can be replaced by
> > > >> > > > >
> > > >> > > > > @pytest.mark.skip(reason="demonstrating skipping"))
> > > >> > > > > def test_foo(self)
> > > >> > > > >
> > > >> > > > > Which solution will be better for us?
> > > >> > > > >
> > > >> > > > > >
> > > >> > > > > > On Wed, Jan 1, 2020 at 8:14 PM Kamil BreguĆa <
> > > >> > > > kamil.bregula@polidea.com>
> > > >> > > > > > wrote:
> > > >> > > > > >
> > > >> > > > > > > > 1) Do we want to use additional plugins?
> > > >> > > > > > >
> > > >> > > > > > > Yes. We should use the full-power of plugins now.
> > > >> > > > > > >
> > > >> > > > > > > > 2) Do we want to use custom markers?
> > > >> > > > > > >
> > > >> > > > > > > Reply in a separate thread.
> > > >> > > > > > >
> > > >> > > > > > > >3) Do we want to use new assert statement?
> > > >> > > > > > >
> > > >> > > > > > > Reply in a separate thread
> > > >> > > > > > >
> > > >> > > > > > > > 4) Do we want to remove the inheritance from
> > > >> unittest.TestCase?
> > > >> > > > > > >
> > > >> > > > > > > Yes. After dropping support for Airflow 2.0, if possible. I
> > > >> would
> > > >> > > > > prefer to
> > > >> > > > > > > focus on working on new features for Airflow 2.0.
> > > >> > > > > > >
> > > >> > > > > > > > 5) Do we want to use class-less tests?
> > > >> > > > > > >
> > > >> > > > > > > No. Classes allow easy grouping of tests. Even if a file with
> > > >> one
> > > >> > > > > class now
> > > >> > > > > > > exists, a new one may appear in the future.
> > > >> > > > > > >
> > > >> > > > > > > > 6) Do we want to use pytest function instead of current?
> > > >> > > > > > >
> > > >> > > > > > > I feel good about the current functions. However, this is not
> > > >> a
> > > >> > > > serious
> > > >> > > > > > > relationship and I can create a new friendship.
> > > >> > > > > > >
> > > >> > > > > > > > 7) Do we want to use monkeypatch fixture?
> > > >> > > > > > >
> > > >> > > > > > > No. I prefer unittest.mock
> > > >> > > > > > >
> > > >> > > > > > > > 8) Do we want to use the pytest fixtures?
> > > >> > > > > > >
> > > >> > > > > > > No. I prefer classic fixtures, if possible. Their syntax is
> > > >> much
> > > >> > > > > simpler
> > > >> > > > > > > and easier to understand.
> > > >> > > > > > >
> > > >> > > > > > > On Wed, Jan 1, 2020 at 8:10 PM Kamil BreguĆa <
> > > >> > > > > kamil.bregula@polidea.com>
> > > >> > > > > > > wrote:
> > > >> > > > > > >
> > > >> > > > > > > > Hello,
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > We have recently migrated to pytest. Code written
> > > >> according to
> > > >> > > the
> > > >> > > > > > > > standard library - unittest.TestCase is still compatible
> > > >> with
> > > >> > > > pytest.
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > The AIP-21 document did not discuss the migration of
> > > >> current code
> > > >> > > > to
> > > >> > > > > > > > new features, but only discussed the change of runner and
> > > >> > > benefits
> > > >> > > > of
> > > >> > > > > > > > pytest over nosetets.
> > > >> > > > > > > >
> > > >> > > > > > > > Link:
> > > >> > > > > > > >
> > > >> > > > > > >
> > > >> > > > >
> > > >> > > >
> > > >> > >
> > > >> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-27+Migrate+to+pytest
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > As far as I appreciate the many advantages of using this
> > > >> tool,
> > > >> > > but
> > > >> > > > I
> > > >> > > > > am
> > > >> > > > > > > > not sure **whether, how or when we want to start using some
> > > >> > > > > features**. I
> > > >> > > > > > > > prefer to keep the current project conventions in many
> > > >> areas,
> > > >> > > but I
> > > >> > > > > still
> > > >> > > > > > > > love pytest. I think we should establish general
> > > >> conventions on
> > > >> > > > > writing
> > > >> > > > > > > > tests and recommended solutions to known problems. I prefer
> > > >> a
> > > >> > > > > pragmatic
> > > >> > > > > > > > approach, not just the use of features, because now we can
> > > >> use
> > > >> > > it.
> > > >> > > > > > > > Unfortunately, I do not see many benefits from new features.
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > I would not like the code to have many ways of expressing
> > > >> the
> > > >> > > same
> > > >> > > > > > > > solution. For the following reasons:
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > * it can introduce a lack of understanding among new
> > > >> contributors
> > > >> > > > > > > >
> > > >> > > > > > > > * facilitate the understanding and reading of code.
> > > >> > > > > > > >
> > > >> > > > > > > > * creates unnecessary discussion about the preferences of
> > > >> one way
> > > >> > > > > over
> > > >> > > > > > > the
> > > >> > > > > > > > other. Not related to changes.
> > > >> > > > > > > >
> > > >> > > > > > > > * forces an understanding of the complex syntax of some
> > > >> > > solutions.
> > > >> > > > > > > >
> > > >> > > > > > > > * encourages people to rewrite the code, which can generate
> > > >> > > > > unnecessary
> > > >> > > > > > > > work.
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > To establish a full convention, I have prepared a few
> > > >> questions:
> > > >> > > > > > > >
> > > >> > > > > > > > 1) Do we want to use additional plugins when there is no
> > > >> function
> > > >> > > > in
> > > >> > > > > the
> > > >> > > > > > > > standard library e.g. flaky marker, forked marker? This
> > > >> is, in
> > > >> > > my
> > > >> > > > > > > opinion,
> > > >> > > > > > > > one of the big advantages of migrating to pytest.
> > > >> > > > > > > >
> > > >> > > > > > > > 2) Do we want to use custom markers?
> > > >> > > > > > > >
> > > >> > > > > > > > The discussion takes place in a separate thread:
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > >
> > > >> > > > >
> > > >> > > >
> > > >> > >
> > > >> https://lists.apache.org/thread.html/4538437c96f599766005ba7829d0bee1511debb4f53599e0d300a56f%40%3Cdev.airflow.apache.org%3E
> > > >> > > > > > > >
> > > >> > > > > > > > 3) Do we want to use new assert statement?
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > >
> > > >> > > > >
> > > >> > > >
> > > >> > >
> > > >> https://lists.apache.org/thread.html/08b64d3b084c865399f98f6c6f56235ce5329e843d97938e1a8045a5%40%3Cdev.airflow.apache.org%3E
> > > >> > > > > > > >
> > > >> > > > > > > > Based on the discussion with devlist, we want to delay
> > > >> migrations
> > > >> > > > to
> > > >> > > > > the
> > > >> > > > > > > > new assert statement.
> > > >> > > > > > > >
> > > >> > > > > > > > 4) Do we want to remove inheritance from unittest.TestCase?
> > > >> > > > > > > >
> > > >> > > > > > > > 5) Do we want to use class-less tests?
> > > >> > > > > > > >
> > > >> > > > > > > > 6) Do we want to use pytest function instead of current?
> > > >> > > > > > > >
> > > >> > > > > > > > For example:
> > > >> > > > > > > >
> > > >> > > > > > > > Unittest.assertRaises vs pytest.raises
> > > >> > > > > > > >
> > > >> > > > > > > > parametrize vs pytest.mark.parametrize
> > > >> > > > > > > >
> > > >> > > > > > > > unittest.skip[If], vs pytest.mark.skip[If]
> > > >> > > > > > > >
> > > >> > > > > > > > 7) Do we want to use monkeypatch fixture?
> > > >> > > > > > > >
> > > >> > > > > > > > https://docs.pytest.org/en/latest/monkeypatch.html
> > > >> > > > > > > >
> > > >> > > > > > > > 8) Do we want to use the pytest fixtures?
> > > >> > > > > > > >
> > > >> > > > > > > > Do we want to migrate all code to pytest fixtures?
> > > >> > > > > > > >
> > > >> > > > > > > > We are currently using a different style of fixtures. Do we
> > > >> want
> > > >> > > to
> > > >> > > > > give
> > > >> > > > > > > > it up?
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > >
> > > >> > > > >
> > > >> > > >
> > > >> > >
> > > >> https://docs.python.org/3/library/unittest.html#class-and-module-fixtures
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > It is worth asking to think about whether we do not want to
> > > >> > > change
> > > >> > > > > this
> > > >> > > > > > > > convention in the future e.g. after dropping support for
> > > >> 1.10.X.
> > > >> > > We
> > > >> > > > > can
> > > >> > > > > > > > also allow two conventions on a temporary basis, and then
> > > >> migrate
> > > >> > > > to
> > > >> > > > > one
> > > >> > > > > > > at
> > > >> > > > > > > > a later time. For example, we want to migrate to the assert
> > > >> > > > statement
> > > >> > > > > > > after
> > > >> > > > > > > > dropping support for 1.10
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > I hope I found the main differences between the current
> > > >> > > convention
> > > >> > > > > and
> > > >> > > > > > > the
> > > >> > > > > > > > new convention. However, if you missed something, please
> > > >> continue
> > > >> > > > to
> > > >> > > > > > > number.
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > Best regards,
> > > >> > > > > > > >
> > > >> > > > > > > > Kamil
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > >
> > > >> > > > >
> > > >> > > >
> > > >> > >
> > > >> > >
> > > >> > > --
> > > >> > >
> > > >> > > Jarek Potiuk
> > > >> > > Polidea <https://www.polidea.com/> | Principal Software Engineer
> > > >> > >
> > > >> > > M: +48 660 796 129 <+48660796129>
> > > >> > > [image: Polidea] <https://www.polidea.com/>
> > > >> > >
> > > >> >
> > > >> >
> > > >> > --
> > > >> >
> > > >> > Tomasz Urbaszek
> > > >> > Polidea <https://www.polidea.com/> | Software Engineer
> > > >> >
> > > >> > M: +48 505 628 493 <+48505628493>
> > > >> > E: tomasz.urbaszek@polidea.com <to...@polidea.com>
> > > >> >
> > > >> > Unique Tech
> > > >> > Check out our projects! <https://www.polidea.com/our-work>
> > > >>
> > > >
> > > >
> > > > --
> > > >
> > > > Jarek Potiuk
> > > > Polidea <https://www.polidea.com/> | Principal Software Engineer
> > > >
> > > > M: +48 660 796 129 <+48660796129>
> > > > [image: Polidea] <https://www.polidea.com/>
> > > >
> > > >
> > >
> > > --
> > >
> > > Jarek Potiuk
> > > Polidea <https://www.polidea.com/> | Principal Software Engineer
> > >
> > > M: +48 660 796 129 <+48660796129>
> > > [image: Polidea] <https://www.polidea.com/>
> >
>
Re: Test style convention after migration to pytest
Posted by Tomasz Urbaszek <tu...@apache.org>.
Hi all,
I would like to resurrect this discussion as another one was started in https://github.com/apache/airflow/issues/11657#issuecomment-713834942
Main questions were:
> 1) Do we want to use additional plugins when there is no function in
> the standard library e.g. flaky marker, forked marker?
> 2) Do we want to use custom markers?
> 3) Do we want to use new assert statement?
> 4) Do we want to remove inheritance from unittest.TestCase?
> 5) Do we want to use class-less tests?
> 6) Do we want to use pytest function instead of current?
> 7) Do we want to use monkeypatch fixture?
> 8) Do we want to use the pytest fixtures?
Probably any changes we decide on should be enforced after 2.0.
Cheers,
Tomek
On 2020/01/18 01:08:29, Kamil BreguĆa <ka...@polidea.com> wrote:
> Hello,
>
> Sorry for the late reply. Recently, I have been very busy with the
> client project and work on the AIP-21.
>
> Let's start. I split this message into several sections for easier reading.
>
> **Establishment of a convention**
> I will prepare a summary. It is available here:
>
> https://docs.google.com/spreadsheets/d/10qToeTFMxjShBX3xcK6IWr9Sp8Pw7qk35fQTtzpZYYs/edit?usp=sharing
>
> The mailing list does not allow rich content, so I copy the verdicts.
>
> 1) Do we want to use additional plugins when there is no function in
> the standard library e.g. flaky marker, forked marker?YES
> 2) Do we want to use custom markers? YES
> 3) Do we want to use new assert statement? YES
> 4) Do we want to remove inheritance from unittest.TestCase? YES
> 5) Do we want to use class-less tests? NO
> 6) Do we want to use pytest function instead of current? No verdict
> 7) Do we want to use monkeypatch fixture? NO
> 8) Do we want to use the pytest fixtures? YES
>
> We don't have a verdict in question 6, because we have no answer from
> Tomek and Jarek. Can you answer this question? A detailed explanation
> is available.
> https://lists.apache.org/thread.html/7ae7ba0e72d3f0d12f8398a85980c5064b58574caa727c6a974fc628%40%3Cdev.airflow.apache.org%3E
> Should we use the pytest functions or use the standard functions?
>
> **Using the new convention in the current code**
> tl;dr; Two conventions will be accepted simultaneously.
> Long story: As for the time of using the new convention, I'm the only
> one who is worried about whether to allow it now. Other people did not
> share my fears. So I think we should allow the use of the new
> conventions now. We want to automate the introduction of the new
> convention in the future, so I think that the application of the two
> conventions should be allowed temporarily. However, I will describe
> the new conventions, but we do not need to strictly require them until
> the remaining code is migrated.
>
> **Full migration to the new convention**
> tl;dr; We'll do it later.
> Long story: Should we migrate the remaining code now? Tomek prefers
> to wait for the end of work on AIP-21. Kaxil and I also prefer to wait
> for AIP-21 or AIrflow 2.0. Jarek prefers to wait until we finish all
> other works that improve the development environment e.g. pylint. So
> we everybody prefers to wait. I think we can go back to the discussion
> when the pylint is completed or we finish AIP-21. It depends on which
> will be earlier.
>
> Best regards,
> Kamil
>
> On Fri, Jan 17, 2020 at 3:07 PM Jarek Potiuk <Ja...@polidea.com> wrote:
> >
> > Should we resume this :)?
> >
> > On Mon, Jan 6, 2020 at 3:42 PM Jarek Potiuk <Ja...@polidea.com>
> > wrote:
> >
> > > Good idea!
> > >
> > > On Mon, Jan 6, 2020 at 3:12 PM Kamil BreguĆa <ka...@polidea.com>
> > > wrote:
> > >
> > >> Hello,
> > >>
> > >> Now is the holiday period. Some people have not started working yet.
> > >> Others are busy with New Year activities. What do you think about
> > >> Friday?
> > >>
> > >> Best regards,
> > >> Kamil
> > >>
> > >> On Mon, Jan 6, 2020 at 2:48 PM Tomasz Urbaszek
> > >> <to...@polidea.com> wrote:
> > >> >
> > >> > When should we assume that we've reached a consensus?
> > >> >
> > >> > T.
> > >> >
> > >> > On Thu, Jan 2, 2020 at 12:52 AM Jarek Potiuk <Ja...@polidea.com>
> > >> > wrote:
> > >> >
> > >> > > > 1) Do we want to use additional plugins? *Yes*
> > >> > >
> > >> > > 2) Do we want to use custom markers? *Yes. They will help with
> > >> optimising
> > >> > > > our test execution.*
> > >> > >
> > >> > > 3) Do we want to use new assert statement? *Yes*
> > >> > > > 4) Do we want to remove the inheritance from unittest.TestCase? *No
> > >> > > > opinion about it. I am ok with both.*
> > >> > > > 5) Do we want to use class-less tests? *No.*
> > >> > > > 6) Do we want to use pytest function instead of current? *I don't
> > >> > > > understand. Can you explain please?*
> > >> > > > 7) Do we want to use monkeypatch fixture? *No. Mock is better.*
> > >> > > > 8) Do we want to use the pytest fixtures? *Yes.*
> > >> > > >
> > >> > >
> > >> > > Great that we have this discussion now. I also think we should just
> > >> agree
> > >> > > it now and not introduce it "globally".
> > >> > > Once we do it, we should simply add it together new features we
> > >> implement.
> > >> > > We have still pylint to finish as a "non-functional global change"
> > >> and we
> > >> > > should not add new one. It's good to continually improve but one
> > >> thing at a
> > >> > > time.
> > >> > >
> > >> > > BTW Pylint goes well we are down to 243 non-pylint files from 991
> > >> since
> > >> > > May.
> > >> > >
> > >> > > J.
> > >> > >
> > >> > > On Thu, Jan 2, 2020 at 12:40 AM Kaxil Naik <ka...@gmail.com>
> > >> wrote:
> > >> > >
> > >> > > > This is a tough one. Both arguments are reasonable.
> > >> > > >
> > >> > > > I agree at some point we should convert all to use assert. But at
> > >> the
> > >> > > same
> > >> > > > time, we should also focus on adding *more user-facing features *and
> > >> > > spend
> > >> > > > less time on more refactor or similar changes.
> > >> > > >
> > >> > > > So based on that, this might be a low priority. We also need to
> > >> still
> > >> > > > complete AIP-21
> > >> > > > <
> > >> > > >
> > >> > >
> > >> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-21%3A+Changes+in+import+paths
> > >> > > > >
> > >> > > > which is very critical for 2.0.
> > >> > > >
> > >> > > > 1) Do we want to use additional plugins?
> > >> > > >
> > >> > > >
> > >> > > > Yes.
> > >> > > >
> > >> > > > 2) Do we want to use custom markers?
> > >> > > >
> > >> > > >
> > >> > > > Not sure yet. Low Priority for me.
> > >> > > >
> > >> > > > 3) Do we want to use new assert statement?
> > >> > > >
> > >> > > >
> > >> > > > I think new PRs can contain it, shouldn't be a problem as long as
> > >> it is
> > >> > > > documented to avoid confusion.
> > >> > > >
> > >> > > > 4) Do we want to remove the inheritance from unittest.TestCase?
> > >> > > >
> > >> > > >
> > >> > > > Yes, this is, however, going to change how people write tests. So
> > >> someone
> > >> > > > has to own it as it can become painful with PRs getting merged
> > >> > > > continuously.
> > >> > > >
> > >> > > > 5) Do we want to use class-less tests?
> > >> > > >
> > >> > > >
> > >> > > > No.
> > >> > > >
> > >> > > > 6) Do we want to use pytest function instead of current?
> > >> > > >
> > >> > > >
> > >> > > > Yes
> > >> > > >
> > >> > > > 7) Do we want to use monkeypatch fixture?
> > >> > > >
> > >> > > >
> > >> > > > I also prefer unittest.mock but open to suggestions.
> > >> > > >
> > >> https://github.com/pytest-dev/pytest/issues/4576#issuecomment-449864333
> > >> > > > has
> > >> > > > some good comparison on it
> > >> > > >
> > >> > > > 8) Do we want to use the pytest fixtures?
> > >> > > >
> > >> > > >
> > >> > > > Yes
> > >> > > >
> > >> > > > On Wed, Jan 1, 2020 at 8:07 PM Kamil BreguĆa <
> > >> kamil.bregula@polidea.com>
> > >> > > > wrote:
> > >> > > >
> > >> > > > > @unittest.skip("demonstrating skipping")
> > >> > > > >
> > >> > > > > On Wed, Jan 1, 2020 at 8:37 PM Tomasz Urbaszek <
> > >> turbaszek@apache.org>
> > >> > > > > wrote:
> > >> > > > > >
> > >> > > > > >
> > >> > > > > > > 6) Do we want to use pytest function instead of current?
> > >> > > > > >
> > >> > > > > > I do not understand this point, can you explain?
> > >> > > > > >
> > >> > > > >
> > >> > > > > Pytest Introduces solutions that replace solutions that are now
> > >> used
> > >> > > > >
> > >> > > > > For example:
> > >> > > > > def test_foo(self):
> > >> > > > > wtih self.assertRaises(AirflowException):
> > >> > > > > bar()
> > >> > > > >
> > >> > > > > Can be replaced by following code:
> > >> > > > >
> > >> > > > > from pytest import raises
> > >> > > > >
> > >> > > > > wtih raises(AirflowException):
> > >> > > > > bar()
> > >> > > > >
> > >> > > > > OR
> > >> > > > >
> > >> > > > > from parametrize import parametrize
> > >> > > > >
> > >> > > > > @parametrize.expand([
> > >> > > > > (1, 1, ),
> > >> > > > > (2, 2, ),
> > >> > > > > ])
> > >> > > > > def test_foo(self, param_a, param_b);
> > >> > > > > self.assertEqual(param_a, param_b)
> > >> > > > >
> > >> > > > > can be replaced by
> > >> > > > >
> > >> > > > > @pytest.mark.parametrize("param_a,param_b", [(1, 1), (2, 2),])
> > >> > > > > def test_eval(param_a, param_b):
> > >> > > > > assert param_a == param_b
> > >> > > > >
> > >> > > > > OR
> > >> > > > >
> > >> > > > > @unittest.skip("demonstrating skipping")
> > >> > > > > def test_foo(self)
> > >> > > > >
> > >> > > > > can be replaced by
> > >> > > > >
> > >> > > > > @pytest.mark.skip(reason="demonstrating skipping"))
> > >> > > > > def test_foo(self)
> > >> > > > >
> > >> > > > > Which solution will be better for us?
> > >> > > > >
> > >> > > > > >
> > >> > > > > > On Wed, Jan 1, 2020 at 8:14 PM Kamil BreguĆa <
> > >> > > > kamil.bregula@polidea.com>
> > >> > > > > > wrote:
> > >> > > > > >
> > >> > > > > > > > 1) Do we want to use additional plugins?
> > >> > > > > > >
> > >> > > > > > > Yes. We should use the full-power of plugins now.
> > >> > > > > > >
> > >> > > > > > > > 2) Do we want to use custom markers?
> > >> > > > > > >
> > >> > > > > > > Reply in a separate thread.
> > >> > > > > > >
> > >> > > > > > > >3) Do we want to use new assert statement?
> > >> > > > > > >
> > >> > > > > > > Reply in a separate thread
> > >> > > > > > >
> > >> > > > > > > > 4) Do we want to remove the inheritance from
> > >> unittest.TestCase?
> > >> > > > > > >
> > >> > > > > > > Yes. After dropping support for Airflow 2.0, if possible. I
> > >> would
> > >> > > > > prefer to
> > >> > > > > > > focus on working on new features for Airflow 2.0.
> > >> > > > > > >
> > >> > > > > > > > 5) Do we want to use class-less tests?
> > >> > > > > > >
> > >> > > > > > > No. Classes allow easy grouping of tests. Even if a file with
> > >> one
> > >> > > > > class now
> > >> > > > > > > exists, a new one may appear in the future.
> > >> > > > > > >
> > >> > > > > > > > 6) Do we want to use pytest function instead of current?
> > >> > > > > > >
> > >> > > > > > > I feel good about the current functions. However, this is not
> > >> a
> > >> > > > serious
> > >> > > > > > > relationship and I can create a new friendship.
> > >> > > > > > >
> > >> > > > > > > > 7) Do we want to use monkeypatch fixture?
> > >> > > > > > >
> > >> > > > > > > No. I prefer unittest.mock
> > >> > > > > > >
> > >> > > > > > > > 8) Do we want to use the pytest fixtures?
> > >> > > > > > >
> > >> > > > > > > No. I prefer classic fixtures, if possible. Their syntax is
> > >> much
> > >> > > > > simpler
> > >> > > > > > > and easier to understand.
> > >> > > > > > >
> > >> > > > > > > On Wed, Jan 1, 2020 at 8:10 PM Kamil BreguĆa <
> > >> > > > > kamil.bregula@polidea.com>
> > >> > > > > > > wrote:
> > >> > > > > > >
> > >> > > > > > > > Hello,
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > > We have recently migrated to pytest. Code written
> > >> according to
> > >> > > the
> > >> > > > > > > > standard library - unittest.TestCase is still compatible
> > >> with
> > >> > > > pytest.
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > > The AIP-21 document did not discuss the migration of
> > >> current code
> > >> > > > to
> > >> > > > > > > > new features, but only discussed the change of runner and
> > >> > > benefits
> > >> > > > of
> > >> > > > > > > > pytest over nosetets.
> > >> > > > > > > >
> > >> > > > > > > > Link:
> > >> > > > > > > >
> > >> > > > > > >
> > >> > > > >
> > >> > > >
> > >> > >
> > >> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-27+Migrate+to+pytest
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > > As far as I appreciate the many advantages of using this
> > >> tool,
> > >> > > but
> > >> > > > I
> > >> > > > > am
> > >> > > > > > > > not sure **whether, how or when we want to start using some
> > >> > > > > features**. I
> > >> > > > > > > > prefer to keep the current project conventions in many
> > >> areas,
> > >> > > but I
> > >> > > > > still
> > >> > > > > > > > love pytest. I think we should establish general
> > >> conventions on
> > >> > > > > writing
> > >> > > > > > > > tests and recommended solutions to known problems. I prefer
> > >> a
> > >> > > > > pragmatic
> > >> > > > > > > > approach, not just the use of features, because now we can
> > >> use
> > >> > > it.
> > >> > > > > > > > Unfortunately, I do not see many benefits from new features.
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > > I would not like the code to have many ways of expressing
> > >> the
> > >> > > same
> > >> > > > > > > > solution. For the following reasons:
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > > * it can introduce a lack of understanding among new
> > >> contributors
> > >> > > > > > > >
> > >> > > > > > > > * facilitate the understanding and reading of code.
> > >> > > > > > > >
> > >> > > > > > > > * creates unnecessary discussion about the preferences of
> > >> one way
> > >> > > > > over
> > >> > > > > > > the
> > >> > > > > > > > other. Not related to changes.
> > >> > > > > > > >
> > >> > > > > > > > * forces an understanding of the complex syntax of some
> > >> > > solutions.
> > >> > > > > > > >
> > >> > > > > > > > * encourages people to rewrite the code, which can generate
> > >> > > > > unnecessary
> > >> > > > > > > > work.
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > > To establish a full convention, I have prepared a few
> > >> questions:
> > >> > > > > > > >
> > >> > > > > > > > 1) Do we want to use additional plugins when there is no
> > >> function
> > >> > > > in
> > >> > > > > the
> > >> > > > > > > > standard library e.g. flaky marker, forked marker? This
> > >> is, in
> > >> > > my
> > >> > > > > > > opinion,
> > >> > > > > > > > one of the big advantages of migrating to pytest.
> > >> > > > > > > >
> > >> > > > > > > > 2) Do we want to use custom markers?
> > >> > > > > > > >
> > >> > > > > > > > The discussion takes place in a separate thread:
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > >
> > >> > > > >
> > >> > > >
> > >> > >
> > >> https://lists.apache.org/thread.html/4538437c96f599766005ba7829d0bee1511debb4f53599e0d300a56f%40%3Cdev.airflow.apache.org%3E
> > >> > > > > > > >
> > >> > > > > > > > 3) Do we want to use new assert statement?
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > >
> > >> > > > >
> > >> > > >
> > >> > >
> > >> https://lists.apache.org/thread.html/08b64d3b084c865399f98f6c6f56235ce5329e843d97938e1a8045a5%40%3Cdev.airflow.apache.org%3E
> > >> > > > > > > >
> > >> > > > > > > > Based on the discussion with devlist, we want to delay
> > >> migrations
> > >> > > > to
> > >> > > > > the
> > >> > > > > > > > new assert statement.
> > >> > > > > > > >
> > >> > > > > > > > 4) Do we want to remove inheritance from unittest.TestCase?
> > >> > > > > > > >
> > >> > > > > > > > 5) Do we want to use class-less tests?
> > >> > > > > > > >
> > >> > > > > > > > 6) Do we want to use pytest function instead of current?
> > >> > > > > > > >
> > >> > > > > > > > For example:
> > >> > > > > > > >
> > >> > > > > > > > Unittest.assertRaises vs pytest.raises
> > >> > > > > > > >
> > >> > > > > > > > parametrize vs pytest.mark.parametrize
> > >> > > > > > > >
> > >> > > > > > > > unittest.skip[If], vs pytest.mark.skip[If]
> > >> > > > > > > >
> > >> > > > > > > > 7) Do we want to use monkeypatch fixture?
> > >> > > > > > > >
> > >> > > > > > > > https://docs.pytest.org/en/latest/monkeypatch.html
> > >> > > > > > > >
> > >> > > > > > > > 8) Do we want to use the pytest fixtures?
> > >> > > > > > > >
> > >> > > > > > > > Do we want to migrate all code to pytest fixtures?
> > >> > > > > > > >
> > >> > > > > > > > We are currently using a different style of fixtures. Do we
> > >> want
> > >> > > to
> > >> > > > > give
> > >> > > > > > > > it up?
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > >
> > >> > > > >
> > >> > > >
> > >> > >
> > >> https://docs.python.org/3/library/unittest.html#class-and-module-fixtures
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > > It is worth asking to think about whether we do not want to
> > >> > > change
> > >> > > > > this
> > >> > > > > > > > convention in the future e.g. after dropping support for
> > >> 1.10.X.
> > >> > > We
> > >> > > > > can
> > >> > > > > > > > also allow two conventions on a temporary basis, and then
> > >> migrate
> > >> > > > to
> > >> > > > > one
> > >> > > > > > > at
> > >> > > > > > > > a later time. For example, we want to migrate to the assert
> > >> > > > statement
> > >> > > > > > > after
> > >> > > > > > > > dropping support for 1.10
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > > I hope I found the main differences between the current
> > >> > > convention
> > >> > > > > and
> > >> > > > > > > the
> > >> > > > > > > > new convention. However, if you missed something, please
> > >> continue
> > >> > > > to
> > >> > > > > > > number.
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > > Best regards,
> > >> > > > > > > >
> > >> > > > > > > > Kamil
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > >
> > >> > > > >
> > >> > > >
> > >> > >
> > >> > >
> > >> > > --
> > >> > >
> > >> > > Jarek Potiuk
> > >> > > Polidea <https://www.polidea.com/> | Principal Software Engineer
> > >> > >
> > >> > > M: +48 660 796 129 <+48660796129>
> > >> > > [image: Polidea] <https://www.polidea.com/>
> > >> > >
> > >> >
> > >> >
> > >> > --
> > >> >
> > >> > Tomasz Urbaszek
> > >> > Polidea <https://www.polidea.com/> | Software Engineer
> > >> >
> > >> > M: +48 505 628 493 <+48505628493>
> > >> > E: tomasz.urbaszek@polidea.com <to...@polidea.com>
> > >> >
> > >> > Unique Tech
> > >> > Check out our projects! <https://www.polidea.com/our-work>
> > >>
> > >
> > >
> > > --
> > >
> > > Jarek Potiuk
> > > Polidea <https://www.polidea.com/> | Principal Software Engineer
> > >
> > > M: +48 660 796 129 <+48660796129>
> > > [image: Polidea] <https://www.polidea.com/>
> > >
> > >
> >
> > --
> >
> > Jarek Potiuk
> > Polidea <https://www.polidea.com/> | Principal Software Engineer
> >
> > M: +48 660 796 129 <+48660796129>
> > [image: Polidea] <https://www.polidea.com/>
>
Re: Test style convention after migration to pytest
Posted by Tomasz Urbaszek <tu...@apache.org>.
Thank you Kamil! I also give +1 for using pytest functions.
T.
On Sat, 18 Jan 2020 at 12:16, Jarek Potiuk <Ja...@polidea.com> wrote:
> Thanks Kamil!
>
> Very much appreciated! It's super clearly explained and thoughtful. Fully
> agree with your assessment.
>
> From my side 6. YES. we should use Pytest functions.
>
> J
>
>
>
> On Sat, Jan 18, 2020 at 2:08 AM Kamil BreguĆa <ka...@polidea.com>
> wrote:
>
> > Hello,
> >
> > Sorry for the late reply. Recently, I have been very busy with the
> > client project and work on the AIP-21.
> >
> > Let's start. I split this message into several sections for easier
> reading.
> >
> > **Establishment of a convention**
> > I will prepare a summary. It is available here:
> >
> >
> >
> https://docs.google.com/spreadsheets/d/10qToeTFMxjShBX3xcK6IWr9Sp8Pw7qk35fQTtzpZYYs/edit?usp=sharing
> >
> > The mailing list does not allow rich content, so I copy the verdicts.
> >
> > 1) Do we want to use additional plugins when there is no function in
> > the standard library e.g. flaky marker, forked marker?YES
> > 2) Do we want to use custom markers? YES
> > 3) Do we want to use new assert statement? YES
> > 4) Do we want to remove inheritance from unittest.TestCase? YES
> > 5) Do we want to use class-less tests? NO
> > 6) Do we want to use pytest function instead of current? No verdict
> > 7) Do we want to use monkeypatch fixture? NO
> > 8) Do we want to use the pytest fixtures? YES
> >
> > We don't have a verdict in question 6, because we have no answer from
> > Tomek and Jarek. Can you answer this question? A detailed explanation
> > is available.
> >
> >
> https://lists.apache.org/thread.html/7ae7ba0e72d3f0d12f8398a85980c5064b58574caa727c6a974fc628%40%3Cdev.airflow.apache.org%3E
> > Should we use the pytest functions or use the standard functions?
> >
> > **Using the new convention in the current code**
> > tl;dr; Two conventions will be accepted simultaneously.
> > Long story: As for the time of using the new convention, I'm the only
> > one who is worried about whether to allow it now. Other people did not
> > share my fears. So I think we should allow the use of the new
> > conventions now. We want to automate the introduction of the new
> > convention in the future, so I think that the application of the two
> > conventions should be allowed temporarily. However, I will describe
> > the new conventions, but we do not need to strictly require them until
> > the remaining code is migrated.
> >
> > **Full migration to the new convention**
> > tl;dr; We'll do it later.
> > Long story: Should we migrate the remaining code now? Tomek prefers
> > to wait for the end of work on AIP-21. Kaxil and I also prefer to wait
> > for AIP-21 or AIrflow 2.0. Jarek prefers to wait until we finish all
> > other works that improve the development environment e.g. pylint. So
> > we everybody prefers to wait. I think we can go back to the discussion
> > when the pylint is completed or we finish AIP-21. It depends on which
> > will be earlier.
> >
> > Best regards,
> > Kamil
> >
> > On Fri, Jan 17, 2020 at 3:07 PM Jarek Potiuk <Ja...@polidea.com>
> > wrote:
> > >
> > > Should we resume this :)?
> > >
> > > On Mon, Jan 6, 2020 at 3:42 PM Jarek Potiuk <Ja...@polidea.com>
> > > wrote:
> > >
> > > > Good idea!
> > > >
> > > > On Mon, Jan 6, 2020 at 3:12 PM Kamil BreguĆa <
> > kamil.bregula@polidea.com>
> > > > wrote:
> > > >
> > > >> Hello,
> > > >>
> > > >> Now is the holiday period. Some people have not started working yet.
> > > >> Others are busy with New Year activities. What do you think about
> > > >> Friday?
> > > >>
> > > >> Best regards,
> > > >> Kamil
> > > >>
> > > >> On Mon, Jan 6, 2020 at 2:48 PM Tomasz Urbaszek
> > > >> <to...@polidea.com> wrote:
> > > >> >
> > > >> > When should we assume that we've reached a consensus?
> > > >> >
> > > >> > T.
> > > >> >
> > > >> > On Thu, Jan 2, 2020 at 12:52 AM Jarek Potiuk <
> > Jarek.Potiuk@polidea.com>
> > > >> > wrote:
> > > >> >
> > > >> > > > 1) Do we want to use additional plugins? *Yes*
> > > >> > >
> > > >> > > 2) Do we want to use custom markers? *Yes. They will help with
> > > >> optimising
> > > >> > > > our test execution.*
> > > >> > >
> > > >> > > 3) Do we want to use new assert statement? *Yes*
> > > >> > > > 4) Do we want to remove the inheritance from
> unittest.TestCase?
> > *No
> > > >> > > > opinion about it. I am ok with both.*
> > > >> > > > 5) Do we want to use class-less tests? *No.*
> > > >> > > > 6) Do we want to use pytest function instead of current? *I
> > don't
> > > >> > > > understand. Can you explain please?*
> > > >> > > > 7) Do we want to use monkeypatch fixture? *No. Mock is
> better.*
> > > >> > > > 8) Do we want to use the pytest fixtures? *Yes.*
> > > >> > > >
> > > >> > >
> > > >> > > Great that we have this discussion now. I also think we should
> > just
> > > >> agree
> > > >> > > it now and not introduce it "globally".
> > > >> > > Once we do it, we should simply add it together new features we
> > > >> implement.
> > > >> > > We have still pylint to finish as a "non-functional global
> change"
> > > >> and we
> > > >> > > should not add new one. It's good to continually improve but one
> > > >> thing at a
> > > >> > > time.
> > > >> > >
> > > >> > > BTW Pylint goes well we are down to 243 non-pylint files from
> 991
> > > >> since
> > > >> > > May.
> > > >> > >
> > > >> > > J.
> > > >> > >
> > > >> > > On Thu, Jan 2, 2020 at 12:40 AM Kaxil Naik <kaxilnaik@gmail.com
> >
> > > >> wrote:
> > > >> > >
> > > >> > > > This is a tough one. Both arguments are reasonable.
> > > >> > > >
> > > >> > > > I agree at some point we should convert all to use assert. But
> > at
> > > >> the
> > > >> > > same
> > > >> > > > time, we should also focus on adding *more user-facing
> features
> > *and
> > > >> > > spend
> > > >> > > > less time on more refactor or similar changes.
> > > >> > > >
> > > >> > > > So based on that, this might be a low priority. We also need
> to
> > > >> still
> > > >> > > > complete AIP-21
> > > >> > > > <
> > > >> > > >
> > > >> > >
> > > >>
> >
> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-21%3A+Changes+in+import+paths
> > > >> > > > >
> > > >> > > > which is very critical for 2.0.
> > > >> > > >
> > > >> > > > 1) Do we want to use additional plugins?
> > > >> > > >
> > > >> > > >
> > > >> > > > Yes.
> > > >> > > >
> > > >> > > > 2) Do we want to use custom markers?
> > > >> > > >
> > > >> > > >
> > > >> > > > Not sure yet. Low Priority for me.
> > > >> > > >
> > > >> > > > 3) Do we want to use new assert statement?
> > > >> > > >
> > > >> > > >
> > > >> > > > I think new PRs can contain it, shouldn't be a problem as long
> > as
> > > >> it is
> > > >> > > > documented to avoid confusion.
> > > >> > > >
> > > >> > > > 4) Do we want to remove the inheritance from
> unittest.TestCase?
> > > >> > > >
> > > >> > > >
> > > >> > > > Yes, this is, however, going to change how people write tests.
> > So
> > > >> someone
> > > >> > > > has to own it as it can become painful with PRs getting merged
> > > >> > > > continuously.
> > > >> > > >
> > > >> > > > 5) Do we want to use class-less tests?
> > > >> > > >
> > > >> > > >
> > > >> > > > No.
> > > >> > > >
> > > >> > > > 6) Do we want to use pytest function instead of current?
> > > >> > > >
> > > >> > > >
> > > >> > > > Yes
> > > >> > > >
> > > >> > > > 7) Do we want to use monkeypatch fixture?
> > > >> > > >
> > > >> > > >
> > > >> > > > I also prefer unittest.mock but open to suggestions.
> > > >> > > >
> > > >>
> > https://github.com/pytest-dev/pytest/issues/4576#issuecomment-449864333
> > > >> > > > has
> > > >> > > > some good comparison on it
> > > >> > > >
> > > >> > > > 8) Do we want to use the pytest fixtures?
> > > >> > > >
> > > >> > > >
> > > >> > > > Yes
> > > >> > > >
> > > >> > > > On Wed, Jan 1, 2020 at 8:07 PM Kamil BreguĆa <
> > > >> kamil.bregula@polidea.com>
> > > >> > > > wrote:
> > > >> > > >
> > > >> > > > > @unittest.skip("demonstrating skipping")
> > > >> > > > >
> > > >> > > > > On Wed, Jan 1, 2020 at 8:37 PM Tomasz Urbaszek <
> > > >> turbaszek@apache.org>
> > > >> > > > > wrote:
> > > >> > > > > >
> > > >> > > > > >
> > > >> > > > > > > 6) Do we want to use pytest function instead of current?
> > > >> > > > > >
> > > >> > > > > > I do not understand this point, can you explain?
> > > >> > > > > >
> > > >> > > > >
> > > >> > > > > Pytest Introduces solutions that replace solutions that are
> > now
> > > >> used
> > > >> > > > >
> > > >> > > > > For example:
> > > >> > > > > def test_foo(self):
> > > >> > > > > wtih self.assertRaises(AirflowException):
> > > >> > > > > bar()
> > > >> > > > >
> > > >> > > > > Can be replaced by following code:
> > > >> > > > >
> > > >> > > > > from pytest import raises
> > > >> > > > >
> > > >> > > > > wtih raises(AirflowException):
> > > >> > > > > bar()
> > > >> > > > >
> > > >> > > > > OR
> > > >> > > > >
> > > >> > > > > from parametrize import parametrize
> > > >> > > > >
> > > >> > > > > @parametrize.expand([
> > > >> > > > > (1, 1, ),
> > > >> > > > > (2, 2, ),
> > > >> > > > > ])
> > > >> > > > > def test_foo(self, param_a, param_b);
> > > >> > > > > self.assertEqual(param_a, param_b)
> > > >> > > > >
> > > >> > > > > can be replaced by
> > > >> > > > >
> > > >> > > > > @pytest.mark.parametrize("param_a,param_b", [(1, 1), (2,
> 2),])
> > > >> > > > > def test_eval(param_a, param_b):
> > > >> > > > > assert param_a == param_b
> > > >> > > > >
> > > >> > > > > OR
> > > >> > > > >
> > > >> > > > > @unittest.skip("demonstrating skipping")
> > > >> > > > > def test_foo(self)
> > > >> > > > >
> > > >> > > > > can be replaced by
> > > >> > > > >
> > > >> > > > > @pytest.mark.skip(reason="demonstrating skipping"))
> > > >> > > > > def test_foo(self)
> > > >> > > > >
> > > >> > > > > Which solution will be better for us?
> > > >> > > > >
> > > >> > > > > >
> > > >> > > > > > On Wed, Jan 1, 2020 at 8:14 PM Kamil BreguĆa <
> > > >> > > > kamil.bregula@polidea.com>
> > > >> > > > > > wrote:
> > > >> > > > > >
> > > >> > > > > > > > 1) Do we want to use additional plugins?
> > > >> > > > > > >
> > > >> > > > > > > Yes. We should use the full-power of plugins now.
> > > >> > > > > > >
> > > >> > > > > > > > 2) Do we want to use custom markers?
> > > >> > > > > > >
> > > >> > > > > > > Reply in a separate thread.
> > > >> > > > > > >
> > > >> > > > > > > >3) Do we want to use new assert statement?
> > > >> > > > > > >
> > > >> > > > > > > Reply in a separate thread
> > > >> > > > > > >
> > > >> > > > > > > > 4) Do we want to remove the inheritance from
> > > >> unittest.TestCase?
> > > >> > > > > > >
> > > >> > > > > > > Yes. After dropping support for Airflow 2.0, if
> possible.
> > I
> > > >> would
> > > >> > > > > prefer to
> > > >> > > > > > > focus on working on new features for Airflow 2.0.
> > > >> > > > > > >
> > > >> > > > > > > > 5) Do we want to use class-less tests?
> > > >> > > > > > >
> > > >> > > > > > > No. Classes allow easy grouping of tests. Even if a file
> > with
> > > >> one
> > > >> > > > > class now
> > > >> > > > > > > exists, a new one may appear in the future.
> > > >> > > > > > >
> > > >> > > > > > > > 6) Do we want to use pytest function instead of
> current?
> > > >> > > > > > >
> > > >> > > > > > > I feel good about the current functions. However, this
> is
> > not
> > > >> a
> > > >> > > > serious
> > > >> > > > > > > relationship and I can create a new friendship.
> > > >> > > > > > >
> > > >> > > > > > > > 7) Do we want to use monkeypatch fixture?
> > > >> > > > > > >
> > > >> > > > > > > No. I prefer unittest.mock
> > > >> > > > > > >
> > > >> > > > > > > > 8) Do we want to use the pytest fixtures?
> > > >> > > > > > >
> > > >> > > > > > > No. I prefer classic fixtures, if possible. Their syntax
> > is
> > > >> much
> > > >> > > > > simpler
> > > >> > > > > > > and easier to understand.
> > > >> > > > > > >
> > > >> > > > > > > On Wed, Jan 1, 2020 at 8:10 PM Kamil BreguĆa <
> > > >> > > > > kamil.bregula@polidea.com>
> > > >> > > > > > > wrote:
> > > >> > > > > > >
> > > >> > > > > > > > Hello,
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > We have recently migrated to pytest. Code written
> > > >> according to
> > > >> > > the
> > > >> > > > > > > > standard library - unittest.TestCase is still
> compatible
> > > >> with
> > > >> > > > pytest.
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > The AIP-21 document did not discuss the migration of
> > > >> current code
> > > >> > > > to
> > > >> > > > > > > > new features, but only discussed the change of runner
> > and
> > > >> > > benefits
> > > >> > > > of
> > > >> > > > > > > > pytest over nosetets.
> > > >> > > > > > > >
> > > >> > > > > > > > Link:
> > > >> > > > > > > >
> > > >> > > > > > >
> > > >> > > > >
> > > >> > > >
> > > >> > >
> > > >>
> >
> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-27+Migrate+to+pytest
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > As far as I appreciate the many advantages of using
> this
> > > >> tool,
> > > >> > > but
> > > >> > > > I
> > > >> > > > > am
> > > >> > > > > > > > not sure **whether, how or when we want to start using
> > some
> > > >> > > > > features**. I
> > > >> > > > > > > > prefer to keep the current project conventions in many
> > > >> areas,
> > > >> > > but I
> > > >> > > > > still
> > > >> > > > > > > > love pytest. I think we should establish general
> > > >> conventions on
> > > >> > > > > writing
> > > >> > > > > > > > tests and recommended solutions to known problems. I
> > prefer
> > > >> a
> > > >> > > > > pragmatic
> > > >> > > > > > > > approach, not just the use of features, because now we
> > can
> > > >> use
> > > >> > > it.
> > > >> > > > > > > > Unfortunately, I do not see many benefits from new
> > features.
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > I would not like the code to have many ways of
> > expressing
> > > >> the
> > > >> > > same
> > > >> > > > > > > > solution. For the following reasons:
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > * it can introduce a lack of understanding among new
> > > >> contributors
> > > >> > > > > > > >
> > > >> > > > > > > > * facilitate the understanding and reading of code.
> > > >> > > > > > > >
> > > >> > > > > > > > * creates unnecessary discussion about the preferences
> > of
> > > >> one way
> > > >> > > > > over
> > > >> > > > > > > the
> > > >> > > > > > > > other. Not related to changes.
> > > >> > > > > > > >
> > > >> > > > > > > > * forces an understanding of the complex syntax of
> some
> > > >> > > solutions.
> > > >> > > > > > > >
> > > >> > > > > > > > * encourages people to rewrite the code, which can
> > generate
> > > >> > > > > unnecessary
> > > >> > > > > > > > work.
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > To establish a full convention, I have prepared a few
> > > >> questions:
> > > >> > > > > > > >
> > > >> > > > > > > > 1) Do we want to use additional plugins when there is
> no
> > > >> function
> > > >> > > > in
> > > >> > > > > the
> > > >> > > > > > > > standard library e.g. flaky marker, forked marker?
> This
> > > >> is, in
> > > >> > > my
> > > >> > > > > > > opinion,
> > > >> > > > > > > > one of the big advantages of migrating to pytest.
> > > >> > > > > > > >
> > > >> > > > > > > > 2) Do we want to use custom markers?
> > > >> > > > > > > >
> > > >> > > > > > > > The discussion takes place in a separate thread:
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > >
> > > >> > > > >
> > > >> > > >
> > > >> > >
> > > >>
> >
> https://lists.apache.org/thread.html/4538437c96f599766005ba7829d0bee1511debb4f53599e0d300a56f%40%3Cdev.airflow.apache.org%3E
> > > >> > > > > > > >
> > > >> > > > > > > > 3) Do we want to use new assert statement?
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > >
> > > >> > > > >
> > > >> > > >
> > > >> > >
> > > >>
> >
> https://lists.apache.org/thread.html/08b64d3b084c865399f98f6c6f56235ce5329e843d97938e1a8045a5%40%3Cdev.airflow.apache.org%3E
> > > >> > > > > > > >
> > > >> > > > > > > > Based on the discussion with devlist, we want to delay
> > > >> migrations
> > > >> > > > to
> > > >> > > > > the
> > > >> > > > > > > > new assert statement.
> > > >> > > > > > > >
> > > >> > > > > > > > 4) Do we want to remove inheritance from
> > unittest.TestCase?
> > > >> > > > > > > >
> > > >> > > > > > > > 5) Do we want to use class-less tests?
> > > >> > > > > > > >
> > > >> > > > > > > > 6) Do we want to use pytest function instead of
> current?
> > > >> > > > > > > >
> > > >> > > > > > > > For example:
> > > >> > > > > > > >
> > > >> > > > > > > > Unittest.assertRaises vs pytest.raises
> > > >> > > > > > > >
> > > >> > > > > > > > parametrize vs pytest.mark.parametrize
> > > >> > > > > > > >
> > > >> > > > > > > > unittest.skip[If], vs pytest.mark.skip[If]
> > > >> > > > > > > >
> > > >> > > > > > > > 7) Do we want to use monkeypatch fixture?
> > > >> > > > > > > >
> > > >> > > > > > > > https://docs.pytest.org/en/latest/monkeypatch.html
> > > >> > > > > > > >
> > > >> > > > > > > > 8) Do we want to use the pytest fixtures?
> > > >> > > > > > > >
> > > >> > > > > > > > Do we want to migrate all code to pytest fixtures?
> > > >> > > > > > > >
> > > >> > > > > > > > We are currently using a different style of fixtures.
> > Do we
> > > >> want
> > > >> > > to
> > > >> > > > > give
> > > >> > > > > > > > it up?
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > >
> > > >> > > > >
> > > >> > > >
> > > >> > >
> > > >>
> >
> https://docs.python.org/3/library/unittest.html#class-and-module-fixtures
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > It is worth asking to think about whether we do not
> > want to
> > > >> > > change
> > > >> > > > > this
> > > >> > > > > > > > convention in the future e.g. after dropping support
> for
> > > >> 1.10.X.
> > > >> > > We
> > > >> > > > > can
> > > >> > > > > > > > also allow two conventions on a temporary basis, and
> > then
> > > >> migrate
> > > >> > > > to
> > > >> > > > > one
> > > >> > > > > > > at
> > > >> > > > > > > > a later time. For example, we want to migrate to the
> > assert
> > > >> > > > statement
> > > >> > > > > > > after
> > > >> > > > > > > > dropping support for 1.10
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > I hope I found the main differences between the
> current
> > > >> > > convention
> > > >> > > > > and
> > > >> > > > > > > the
> > > >> > > > > > > > new convention. However, if you missed something,
> please
> > > >> continue
> > > >> > > > to
> > > >> > > > > > > number.
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > Best regards,
> > > >> > > > > > > >
> > > >> > > > > > > > Kamil
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > >
> > > >> > > > >
> > > >> > > >
> > > >> > >
> > > >> > >
> > > >> > > --
> > > >> > >
> > > >> > > Jarek Potiuk
> > > >> > > Polidea <https://www.polidea.com/> | Principal Software
> Engineer
> > > >> > >
> > > >> > > M: +48 660 796 129 <+48660796129>
> > > >> > > [image: Polidea] <https://www.polidea.com/>
> > > >> > >
> > > >> >
> > > >> >
> > > >> > --
> > > >> >
> > > >> > Tomasz Urbaszek
> > > >> > Polidea <https://www.polidea.com/> | Software Engineer
> > > >> >
> > > >> > M: +48 505 628 493 <+48505628493>
> > > >> > E: tomasz.urbaszek@polidea.com <to...@polidea.com>
> > > >> >
> > > >> > Unique Tech
> > > >> > Check out our projects! <https://www.polidea.com/our-work>
> > > >>
> > > >
> > > >
> > > > --
> > > >
> > > > Jarek Potiuk
> > > > Polidea <https://www.polidea.com/> | Principal Software Engineer
> > > >
> > > > M: +48 660 796 129 <+48660796129>
> > > > [image: Polidea] <https://www.polidea.com/>
> > > >
> > > >
> > >
> > > --
> > >
> > > Jarek Potiuk
> > > Polidea <https://www.polidea.com/> | Principal Software Engineer
> > >
> > > M: +48 660 796 129 <+48660796129>
> > > [image: Polidea] <https://www.polidea.com/>
> >
>
>
> --
>
> Jarek Potiuk
> Polidea <https://www.polidea.com/> | Principal Software Engineer
>
> M: +48 660 796 129 <+48660796129>
> [image: Polidea] <https://www.polidea.com/>
>
Re: Test style convention after migration to pytest
Posted by Jarek Potiuk <Ja...@polidea.com>.
Thanks Kamil!
Very much appreciated! It's super clearly explained and thoughtful. Fully
agree with your assessment.
From my side 6. YES. we should use Pytest functions.
J
On Sat, Jan 18, 2020 at 2:08 AM Kamil BreguĆa <ka...@polidea.com>
wrote:
> Hello,
>
> Sorry for the late reply. Recently, I have been very busy with the
> client project and work on the AIP-21.
>
> Let's start. I split this message into several sections for easier reading.
>
> **Establishment of a convention**
> I will prepare a summary. It is available here:
>
>
> https://docs.google.com/spreadsheets/d/10qToeTFMxjShBX3xcK6IWr9Sp8Pw7qk35fQTtzpZYYs/edit?usp=sharing
>
> The mailing list does not allow rich content, so I copy the verdicts.
>
> 1) Do we want to use additional plugins when there is no function in
> the standard library e.g. flaky marker, forked marker?YES
> 2) Do we want to use custom markers? YES
> 3) Do we want to use new assert statement? YES
> 4) Do we want to remove inheritance from unittest.TestCase? YES
> 5) Do we want to use class-less tests? NO
> 6) Do we want to use pytest function instead of current? No verdict
> 7) Do we want to use monkeypatch fixture? NO
> 8) Do we want to use the pytest fixtures? YES
>
> We don't have a verdict in question 6, because we have no answer from
> Tomek and Jarek. Can you answer this question? A detailed explanation
> is available.
>
> https://lists.apache.org/thread.html/7ae7ba0e72d3f0d12f8398a85980c5064b58574caa727c6a974fc628%40%3Cdev.airflow.apache.org%3E
> Should we use the pytest functions or use the standard functions?
>
> **Using the new convention in the current code**
> tl;dr; Two conventions will be accepted simultaneously.
> Long story: As for the time of using the new convention, I'm the only
> one who is worried about whether to allow it now. Other people did not
> share my fears. So I think we should allow the use of the new
> conventions now. We want to automate the introduction of the new
> convention in the future, so I think that the application of the two
> conventions should be allowed temporarily. However, I will describe
> the new conventions, but we do not need to strictly require them until
> the remaining code is migrated.
>
> **Full migration to the new convention**
> tl;dr; We'll do it later.
> Long story: Should we migrate the remaining code now? Tomek prefers
> to wait for the end of work on AIP-21. Kaxil and I also prefer to wait
> for AIP-21 or AIrflow 2.0. Jarek prefers to wait until we finish all
> other works that improve the development environment e.g. pylint. So
> we everybody prefers to wait. I think we can go back to the discussion
> when the pylint is completed or we finish AIP-21. It depends on which
> will be earlier.
>
> Best regards,
> Kamil
>
> On Fri, Jan 17, 2020 at 3:07 PM Jarek Potiuk <Ja...@polidea.com>
> wrote:
> >
> > Should we resume this :)?
> >
> > On Mon, Jan 6, 2020 at 3:42 PM Jarek Potiuk <Ja...@polidea.com>
> > wrote:
> >
> > > Good idea!
> > >
> > > On Mon, Jan 6, 2020 at 3:12 PM Kamil BreguĆa <
> kamil.bregula@polidea.com>
> > > wrote:
> > >
> > >> Hello,
> > >>
> > >> Now is the holiday period. Some people have not started working yet.
> > >> Others are busy with New Year activities. What do you think about
> > >> Friday?
> > >>
> > >> Best regards,
> > >> Kamil
> > >>
> > >> On Mon, Jan 6, 2020 at 2:48 PM Tomasz Urbaszek
> > >> <to...@polidea.com> wrote:
> > >> >
> > >> > When should we assume that we've reached a consensus?
> > >> >
> > >> > T.
> > >> >
> > >> > On Thu, Jan 2, 2020 at 12:52 AM Jarek Potiuk <
> Jarek.Potiuk@polidea.com>
> > >> > wrote:
> > >> >
> > >> > > > 1) Do we want to use additional plugins? *Yes*
> > >> > >
> > >> > > 2) Do we want to use custom markers? *Yes. They will help with
> > >> optimising
> > >> > > > our test execution.*
> > >> > >
> > >> > > 3) Do we want to use new assert statement? *Yes*
> > >> > > > 4) Do we want to remove the inheritance from unittest.TestCase?
> *No
> > >> > > > opinion about it. I am ok with both.*
> > >> > > > 5) Do we want to use class-less tests? *No.*
> > >> > > > 6) Do we want to use pytest function instead of current? *I
> don't
> > >> > > > understand. Can you explain please?*
> > >> > > > 7) Do we want to use monkeypatch fixture? *No. Mock is better.*
> > >> > > > 8) Do we want to use the pytest fixtures? *Yes.*
> > >> > > >
> > >> > >
> > >> > > Great that we have this discussion now. I also think we should
> just
> > >> agree
> > >> > > it now and not introduce it "globally".
> > >> > > Once we do it, we should simply add it together new features we
> > >> implement.
> > >> > > We have still pylint to finish as a "non-functional global change"
> > >> and we
> > >> > > should not add new one. It's good to continually improve but one
> > >> thing at a
> > >> > > time.
> > >> > >
> > >> > > BTW Pylint goes well we are down to 243 non-pylint files from 991
> > >> since
> > >> > > May.
> > >> > >
> > >> > > J.
> > >> > >
> > >> > > On Thu, Jan 2, 2020 at 12:40 AM Kaxil Naik <ka...@gmail.com>
> > >> wrote:
> > >> > >
> > >> > > > This is a tough one. Both arguments are reasonable.
> > >> > > >
> > >> > > > I agree at some point we should convert all to use assert. But
> at
> > >> the
> > >> > > same
> > >> > > > time, we should also focus on adding *more user-facing features
> *and
> > >> > > spend
> > >> > > > less time on more refactor or similar changes.
> > >> > > >
> > >> > > > So based on that, this might be a low priority. We also need to
> > >> still
> > >> > > > complete AIP-21
> > >> > > > <
> > >> > > >
> > >> > >
> > >>
> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-21%3A+Changes+in+import+paths
> > >> > > > >
> > >> > > > which is very critical for 2.0.
> > >> > > >
> > >> > > > 1) Do we want to use additional plugins?
> > >> > > >
> > >> > > >
> > >> > > > Yes.
> > >> > > >
> > >> > > > 2) Do we want to use custom markers?
> > >> > > >
> > >> > > >
> > >> > > > Not sure yet. Low Priority for me.
> > >> > > >
> > >> > > > 3) Do we want to use new assert statement?
> > >> > > >
> > >> > > >
> > >> > > > I think new PRs can contain it, shouldn't be a problem as long
> as
> > >> it is
> > >> > > > documented to avoid confusion.
> > >> > > >
> > >> > > > 4) Do we want to remove the inheritance from unittest.TestCase?
> > >> > > >
> > >> > > >
> > >> > > > Yes, this is, however, going to change how people write tests.
> So
> > >> someone
> > >> > > > has to own it as it can become painful with PRs getting merged
> > >> > > > continuously.
> > >> > > >
> > >> > > > 5) Do we want to use class-less tests?
> > >> > > >
> > >> > > >
> > >> > > > No.
> > >> > > >
> > >> > > > 6) Do we want to use pytest function instead of current?
> > >> > > >
> > >> > > >
> > >> > > > Yes
> > >> > > >
> > >> > > > 7) Do we want to use monkeypatch fixture?
> > >> > > >
> > >> > > >
> > >> > > > I also prefer unittest.mock but open to suggestions.
> > >> > > >
> > >>
> https://github.com/pytest-dev/pytest/issues/4576#issuecomment-449864333
> > >> > > > has
> > >> > > > some good comparison on it
> > >> > > >
> > >> > > > 8) Do we want to use the pytest fixtures?
> > >> > > >
> > >> > > >
> > >> > > > Yes
> > >> > > >
> > >> > > > On Wed, Jan 1, 2020 at 8:07 PM Kamil BreguĆa <
> > >> kamil.bregula@polidea.com>
> > >> > > > wrote:
> > >> > > >
> > >> > > > > @unittest.skip("demonstrating skipping")
> > >> > > > >
> > >> > > > > On Wed, Jan 1, 2020 at 8:37 PM Tomasz Urbaszek <
> > >> turbaszek@apache.org>
> > >> > > > > wrote:
> > >> > > > > >
> > >> > > > > >
> > >> > > > > > > 6) Do we want to use pytest function instead of current?
> > >> > > > > >
> > >> > > > > > I do not understand this point, can you explain?
> > >> > > > > >
> > >> > > > >
> > >> > > > > Pytest Introduces solutions that replace solutions that are
> now
> > >> used
> > >> > > > >
> > >> > > > > For example:
> > >> > > > > def test_foo(self):
> > >> > > > > wtih self.assertRaises(AirflowException):
> > >> > > > > bar()
> > >> > > > >
> > >> > > > > Can be replaced by following code:
> > >> > > > >
> > >> > > > > from pytest import raises
> > >> > > > >
> > >> > > > > wtih raises(AirflowException):
> > >> > > > > bar()
> > >> > > > >
> > >> > > > > OR
> > >> > > > >
> > >> > > > > from parametrize import parametrize
> > >> > > > >
> > >> > > > > @parametrize.expand([
> > >> > > > > (1, 1, ),
> > >> > > > > (2, 2, ),
> > >> > > > > ])
> > >> > > > > def test_foo(self, param_a, param_b);
> > >> > > > > self.assertEqual(param_a, param_b)
> > >> > > > >
> > >> > > > > can be replaced by
> > >> > > > >
> > >> > > > > @pytest.mark.parametrize("param_a,param_b", [(1, 1), (2, 2),])
> > >> > > > > def test_eval(param_a, param_b):
> > >> > > > > assert param_a == param_b
> > >> > > > >
> > >> > > > > OR
> > >> > > > >
> > >> > > > > @unittest.skip("demonstrating skipping")
> > >> > > > > def test_foo(self)
> > >> > > > >
> > >> > > > > can be replaced by
> > >> > > > >
> > >> > > > > @pytest.mark.skip(reason="demonstrating skipping"))
> > >> > > > > def test_foo(self)
> > >> > > > >
> > >> > > > > Which solution will be better for us?
> > >> > > > >
> > >> > > > > >
> > >> > > > > > On Wed, Jan 1, 2020 at 8:14 PM Kamil BreguĆa <
> > >> > > > kamil.bregula@polidea.com>
> > >> > > > > > wrote:
> > >> > > > > >
> > >> > > > > > > > 1) Do we want to use additional plugins?
> > >> > > > > > >
> > >> > > > > > > Yes. We should use the full-power of plugins now.
> > >> > > > > > >
> > >> > > > > > > > 2) Do we want to use custom markers?
> > >> > > > > > >
> > >> > > > > > > Reply in a separate thread.
> > >> > > > > > >
> > >> > > > > > > >3) Do we want to use new assert statement?
> > >> > > > > > >
> > >> > > > > > > Reply in a separate thread
> > >> > > > > > >
> > >> > > > > > > > 4) Do we want to remove the inheritance from
> > >> unittest.TestCase?
> > >> > > > > > >
> > >> > > > > > > Yes. After dropping support for Airflow 2.0, if possible.
> I
> > >> would
> > >> > > > > prefer to
> > >> > > > > > > focus on working on new features for Airflow 2.0.
> > >> > > > > > >
> > >> > > > > > > > 5) Do we want to use class-less tests?
> > >> > > > > > >
> > >> > > > > > > No. Classes allow easy grouping of tests. Even if a file
> with
> > >> one
> > >> > > > > class now
> > >> > > > > > > exists, a new one may appear in the future.
> > >> > > > > > >
> > >> > > > > > > > 6) Do we want to use pytest function instead of current?
> > >> > > > > > >
> > >> > > > > > > I feel good about the current functions. However, this is
> not
> > >> a
> > >> > > > serious
> > >> > > > > > > relationship and I can create a new friendship.
> > >> > > > > > >
> > >> > > > > > > > 7) Do we want to use monkeypatch fixture?
> > >> > > > > > >
> > >> > > > > > > No. I prefer unittest.mock
> > >> > > > > > >
> > >> > > > > > > > 8) Do we want to use the pytest fixtures?
> > >> > > > > > >
> > >> > > > > > > No. I prefer classic fixtures, if possible. Their syntax
> is
> > >> much
> > >> > > > > simpler
> > >> > > > > > > and easier to understand.
> > >> > > > > > >
> > >> > > > > > > On Wed, Jan 1, 2020 at 8:10 PM Kamil BreguĆa <
> > >> > > > > kamil.bregula@polidea.com>
> > >> > > > > > > wrote:
> > >> > > > > > >
> > >> > > > > > > > Hello,
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > > We have recently migrated to pytest. Code written
> > >> according to
> > >> > > the
> > >> > > > > > > > standard library - unittest.TestCase is still compatible
> > >> with
> > >> > > > pytest.
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > > The AIP-21 document did not discuss the migration of
> > >> current code
> > >> > > > to
> > >> > > > > > > > new features, but only discussed the change of runner
> and
> > >> > > benefits
> > >> > > > of
> > >> > > > > > > > pytest over nosetets.
> > >> > > > > > > >
> > >> > > > > > > > Link:
> > >> > > > > > > >
> > >> > > > > > >
> > >> > > > >
> > >> > > >
> > >> > >
> > >>
> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-27+Migrate+to+pytest
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > > As far as I appreciate the many advantages of using this
> > >> tool,
> > >> > > but
> > >> > > > I
> > >> > > > > am
> > >> > > > > > > > not sure **whether, how or when we want to start using
> some
> > >> > > > > features**. I
> > >> > > > > > > > prefer to keep the current project conventions in many
> > >> areas,
> > >> > > but I
> > >> > > > > still
> > >> > > > > > > > love pytest. I think we should establish general
> > >> conventions on
> > >> > > > > writing
> > >> > > > > > > > tests and recommended solutions to known problems. I
> prefer
> > >> a
> > >> > > > > pragmatic
> > >> > > > > > > > approach, not just the use of features, because now we
> can
> > >> use
> > >> > > it.
> > >> > > > > > > > Unfortunately, I do not see many benefits from new
> features.
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > > I would not like the code to have many ways of
> expressing
> > >> the
> > >> > > same
> > >> > > > > > > > solution. For the following reasons:
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > > * it can introduce a lack of understanding among new
> > >> contributors
> > >> > > > > > > >
> > >> > > > > > > > * facilitate the understanding and reading of code.
> > >> > > > > > > >
> > >> > > > > > > > * creates unnecessary discussion about the preferences
> of
> > >> one way
> > >> > > > > over
> > >> > > > > > > the
> > >> > > > > > > > other. Not related to changes.
> > >> > > > > > > >
> > >> > > > > > > > * forces an understanding of the complex syntax of some
> > >> > > solutions.
> > >> > > > > > > >
> > >> > > > > > > > * encourages people to rewrite the code, which can
> generate
> > >> > > > > unnecessary
> > >> > > > > > > > work.
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > > To establish a full convention, I have prepared a few
> > >> questions:
> > >> > > > > > > >
> > >> > > > > > > > 1) Do we want to use additional plugins when there is no
> > >> function
> > >> > > > in
> > >> > > > > the
> > >> > > > > > > > standard library e.g. flaky marker, forked marker? This
> > >> is, in
> > >> > > my
> > >> > > > > > > opinion,
> > >> > > > > > > > one of the big advantages of migrating to pytest.
> > >> > > > > > > >
> > >> > > > > > > > 2) Do we want to use custom markers?
> > >> > > > > > > >
> > >> > > > > > > > The discussion takes place in a separate thread:
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > >
> > >> > > > >
> > >> > > >
> > >> > >
> > >>
> https://lists.apache.org/thread.html/4538437c96f599766005ba7829d0bee1511debb4f53599e0d300a56f%40%3Cdev.airflow.apache.org%3E
> > >> > > > > > > >
> > >> > > > > > > > 3) Do we want to use new assert statement?
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > >
> > >> > > > >
> > >> > > >
> > >> > >
> > >>
> https://lists.apache.org/thread.html/08b64d3b084c865399f98f6c6f56235ce5329e843d97938e1a8045a5%40%3Cdev.airflow.apache.org%3E
> > >> > > > > > > >
> > >> > > > > > > > Based on the discussion with devlist, we want to delay
> > >> migrations
> > >> > > > to
> > >> > > > > the
> > >> > > > > > > > new assert statement.
> > >> > > > > > > >
> > >> > > > > > > > 4) Do we want to remove inheritance from
> unittest.TestCase?
> > >> > > > > > > >
> > >> > > > > > > > 5) Do we want to use class-less tests?
> > >> > > > > > > >
> > >> > > > > > > > 6) Do we want to use pytest function instead of current?
> > >> > > > > > > >
> > >> > > > > > > > For example:
> > >> > > > > > > >
> > >> > > > > > > > Unittest.assertRaises vs pytest.raises
> > >> > > > > > > >
> > >> > > > > > > > parametrize vs pytest.mark.parametrize
> > >> > > > > > > >
> > >> > > > > > > > unittest.skip[If], vs pytest.mark.skip[If]
> > >> > > > > > > >
> > >> > > > > > > > 7) Do we want to use monkeypatch fixture?
> > >> > > > > > > >
> > >> > > > > > > > https://docs.pytest.org/en/latest/monkeypatch.html
> > >> > > > > > > >
> > >> > > > > > > > 8) Do we want to use the pytest fixtures?
> > >> > > > > > > >
> > >> > > > > > > > Do we want to migrate all code to pytest fixtures?
> > >> > > > > > > >
> > >> > > > > > > > We are currently using a different style of fixtures.
> Do we
> > >> want
> > >> > > to
> > >> > > > > give
> > >> > > > > > > > it up?
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > >
> > >> > > > >
> > >> > > >
> > >> > >
> > >>
> https://docs.python.org/3/library/unittest.html#class-and-module-fixtures
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > > It is worth asking to think about whether we do not
> want to
> > >> > > change
> > >> > > > > this
> > >> > > > > > > > convention in the future e.g. after dropping support for
> > >> 1.10.X.
> > >> > > We
> > >> > > > > can
> > >> > > > > > > > also allow two conventions on a temporary basis, and
> then
> > >> migrate
> > >> > > > to
> > >> > > > > one
> > >> > > > > > > at
> > >> > > > > > > > a later time. For example, we want to migrate to the
> assert
> > >> > > > statement
> > >> > > > > > > after
> > >> > > > > > > > dropping support for 1.10
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > > I hope I found the main differences between the current
> > >> > > convention
> > >> > > > > and
> > >> > > > > > > the
> > >> > > > > > > > new convention. However, if you missed something, please
> > >> continue
> > >> > > > to
> > >> > > > > > > number.
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > > Best regards,
> > >> > > > > > > >
> > >> > > > > > > > Kamil
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > >
> > >> > > > >
> > >> > > >
> > >> > >
> > >> > >
> > >> > > --
> > >> > >
> > >> > > Jarek Potiuk
> > >> > > Polidea <https://www.polidea.com/> | Principal Software Engineer
> > >> > >
> > >> > > M: +48 660 796 129 <+48660796129>
> > >> > > [image: Polidea] <https://www.polidea.com/>
> > >> > >
> > >> >
> > >> >
> > >> > --
> > >> >
> > >> > Tomasz Urbaszek
> > >> > Polidea <https://www.polidea.com/> | Software Engineer
> > >> >
> > >> > M: +48 505 628 493 <+48505628493>
> > >> > E: tomasz.urbaszek@polidea.com <to...@polidea.com>
> > >> >
> > >> > Unique Tech
> > >> > Check out our projects! <https://www.polidea.com/our-work>
> > >>
> > >
> > >
> > > --
> > >
> > > Jarek Potiuk
> > > Polidea <https://www.polidea.com/> | Principal Software Engineer
> > >
> > > M: +48 660 796 129 <+48660796129>
> > > [image: Polidea] <https://www.polidea.com/>
> > >
> > >
> >
> > --
> >
> > Jarek Potiuk
> > Polidea <https://www.polidea.com/> | Principal Software Engineer
> >
> > M: +48 660 796 129 <+48660796129>
> > [image: Polidea] <https://www.polidea.com/>
>
--
Jarek Potiuk
Polidea <https://www.polidea.com/> | Principal Software Engineer
M: +48 660 796 129 <+48660796129>
[image: Polidea] <https://www.polidea.com/>
Re: Test style convention after migration to pytest
Posted by Kamil BreguĆa <ka...@polidea.com>.
Hello,
Sorry for the late reply. Recently, I have been very busy with the
client project and work on the AIP-21.
Let's start. I split this message into several sections for easier reading.
**Establishment of a convention**
I will prepare a summary. It is available here:
https://docs.google.com/spreadsheets/d/10qToeTFMxjShBX3xcK6IWr9Sp8Pw7qk35fQTtzpZYYs/edit?usp=sharing
The mailing list does not allow rich content, so I copy the verdicts.
1) Do we want to use additional plugins when there is no function in
the standard library e.g. flaky marker, forked marker?YES
2) Do we want to use custom markers? YES
3) Do we want to use new assert statement? YES
4) Do we want to remove inheritance from unittest.TestCase? YES
5) Do we want to use class-less tests? NO
6) Do we want to use pytest function instead of current? No verdict
7) Do we want to use monkeypatch fixture? NO
8) Do we want to use the pytest fixtures? YES
We don't have a verdict in question 6, because we have no answer from
Tomek and Jarek. Can you answer this question? A detailed explanation
is available.
https://lists.apache.org/thread.html/7ae7ba0e72d3f0d12f8398a85980c5064b58574caa727c6a974fc628%40%3Cdev.airflow.apache.org%3E
Should we use the pytest functions or use the standard functions?
**Using the new convention in the current code**
tl;dr; Two conventions will be accepted simultaneously.
Long story: As for the time of using the new convention, I'm the only
one who is worried about whether to allow it now. Other people did not
share my fears. So I think we should allow the use of the new
conventions now. We want to automate the introduction of the new
convention in the future, so I think that the application of the two
conventions should be allowed temporarily. However, I will describe
the new conventions, but we do not need to strictly require them until
the remaining code is migrated.
**Full migration to the new convention**
tl;dr; We'll do it later.
Long story: Should we migrate the remaining code now? Tomek prefers
to wait for the end of work on AIP-21. Kaxil and I also prefer to wait
for AIP-21 or AIrflow 2.0. Jarek prefers to wait until we finish all
other works that improve the development environment e.g. pylint. So
we everybody prefers to wait. I think we can go back to the discussion
when the pylint is completed or we finish AIP-21. It depends on which
will be earlier.
Best regards,
Kamil
On Fri, Jan 17, 2020 at 3:07 PM Jarek Potiuk <Ja...@polidea.com> wrote:
>
> Should we resume this :)?
>
> On Mon, Jan 6, 2020 at 3:42 PM Jarek Potiuk <Ja...@polidea.com>
> wrote:
>
> > Good idea!
> >
> > On Mon, Jan 6, 2020 at 3:12 PM Kamil BreguĆa <ka...@polidea.com>
> > wrote:
> >
> >> Hello,
> >>
> >> Now is the holiday period. Some people have not started working yet.
> >> Others are busy with New Year activities. What do you think about
> >> Friday?
> >>
> >> Best regards,
> >> Kamil
> >>
> >> On Mon, Jan 6, 2020 at 2:48 PM Tomasz Urbaszek
> >> <to...@polidea.com> wrote:
> >> >
> >> > When should we assume that we've reached a consensus?
> >> >
> >> > T.
> >> >
> >> > On Thu, Jan 2, 2020 at 12:52 AM Jarek Potiuk <Ja...@polidea.com>
> >> > wrote:
> >> >
> >> > > > 1) Do we want to use additional plugins? *Yes*
> >> > >
> >> > > 2) Do we want to use custom markers? *Yes. They will help with
> >> optimising
> >> > > > our test execution.*
> >> > >
> >> > > 3) Do we want to use new assert statement? *Yes*
> >> > > > 4) Do we want to remove the inheritance from unittest.TestCase? *No
> >> > > > opinion about it. I am ok with both.*
> >> > > > 5) Do we want to use class-less tests? *No.*
> >> > > > 6) Do we want to use pytest function instead of current? *I don't
> >> > > > understand. Can you explain please?*
> >> > > > 7) Do we want to use monkeypatch fixture? *No. Mock is better.*
> >> > > > 8) Do we want to use the pytest fixtures? *Yes.*
> >> > > >
> >> > >
> >> > > Great that we have this discussion now. I also think we should just
> >> agree
> >> > > it now and not introduce it "globally".
> >> > > Once we do it, we should simply add it together new features we
> >> implement.
> >> > > We have still pylint to finish as a "non-functional global change"
> >> and we
> >> > > should not add new one. It's good to continually improve but one
> >> thing at a
> >> > > time.
> >> > >
> >> > > BTW Pylint goes well we are down to 243 non-pylint files from 991
> >> since
> >> > > May.
> >> > >
> >> > > J.
> >> > >
> >> > > On Thu, Jan 2, 2020 at 12:40 AM Kaxil Naik <ka...@gmail.com>
> >> wrote:
> >> > >
> >> > > > This is a tough one. Both arguments are reasonable.
> >> > > >
> >> > > > I agree at some point we should convert all to use assert. But at
> >> the
> >> > > same
> >> > > > time, we should also focus on adding *more user-facing features *and
> >> > > spend
> >> > > > less time on more refactor or similar changes.
> >> > > >
> >> > > > So based on that, this might be a low priority. We also need to
> >> still
> >> > > > complete AIP-21
> >> > > > <
> >> > > >
> >> > >
> >> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-21%3A+Changes+in+import+paths
> >> > > > >
> >> > > > which is very critical for 2.0.
> >> > > >
> >> > > > 1) Do we want to use additional plugins?
> >> > > >
> >> > > >
> >> > > > Yes.
> >> > > >
> >> > > > 2) Do we want to use custom markers?
> >> > > >
> >> > > >
> >> > > > Not sure yet. Low Priority for me.
> >> > > >
> >> > > > 3) Do we want to use new assert statement?
> >> > > >
> >> > > >
> >> > > > I think new PRs can contain it, shouldn't be a problem as long as
> >> it is
> >> > > > documented to avoid confusion.
> >> > > >
> >> > > > 4) Do we want to remove the inheritance from unittest.TestCase?
> >> > > >
> >> > > >
> >> > > > Yes, this is, however, going to change how people write tests. So
> >> someone
> >> > > > has to own it as it can become painful with PRs getting merged
> >> > > > continuously.
> >> > > >
> >> > > > 5) Do we want to use class-less tests?
> >> > > >
> >> > > >
> >> > > > No.
> >> > > >
> >> > > > 6) Do we want to use pytest function instead of current?
> >> > > >
> >> > > >
> >> > > > Yes
> >> > > >
> >> > > > 7) Do we want to use monkeypatch fixture?
> >> > > >
> >> > > >
> >> > > > I also prefer unittest.mock but open to suggestions.
> >> > > >
> >> https://github.com/pytest-dev/pytest/issues/4576#issuecomment-449864333
> >> > > > has
> >> > > > some good comparison on it
> >> > > >
> >> > > > 8) Do we want to use the pytest fixtures?
> >> > > >
> >> > > >
> >> > > > Yes
> >> > > >
> >> > > > On Wed, Jan 1, 2020 at 8:07 PM Kamil BreguĆa <
> >> kamil.bregula@polidea.com>
> >> > > > wrote:
> >> > > >
> >> > > > > @unittest.skip("demonstrating skipping")
> >> > > > >
> >> > > > > On Wed, Jan 1, 2020 at 8:37 PM Tomasz Urbaszek <
> >> turbaszek@apache.org>
> >> > > > > wrote:
> >> > > > > >
> >> > > > > >
> >> > > > > > > 6) Do we want to use pytest function instead of current?
> >> > > > > >
> >> > > > > > I do not understand this point, can you explain?
> >> > > > > >
> >> > > > >
> >> > > > > Pytest Introduces solutions that replace solutions that are now
> >> used
> >> > > > >
> >> > > > > For example:
> >> > > > > def test_foo(self):
> >> > > > > wtih self.assertRaises(AirflowException):
> >> > > > > bar()
> >> > > > >
> >> > > > > Can be replaced by following code:
> >> > > > >
> >> > > > > from pytest import raises
> >> > > > >
> >> > > > > wtih raises(AirflowException):
> >> > > > > bar()
> >> > > > >
> >> > > > > OR
> >> > > > >
> >> > > > > from parametrize import parametrize
> >> > > > >
> >> > > > > @parametrize.expand([
> >> > > > > (1, 1, ),
> >> > > > > (2, 2, ),
> >> > > > > ])
> >> > > > > def test_foo(self, param_a, param_b);
> >> > > > > self.assertEqual(param_a, param_b)
> >> > > > >
> >> > > > > can be replaced by
> >> > > > >
> >> > > > > @pytest.mark.parametrize("param_a,param_b", [(1, 1), (2, 2),])
> >> > > > > def test_eval(param_a, param_b):
> >> > > > > assert param_a == param_b
> >> > > > >
> >> > > > > OR
> >> > > > >
> >> > > > > @unittest.skip("demonstrating skipping")
> >> > > > > def test_foo(self)
> >> > > > >
> >> > > > > can be replaced by
> >> > > > >
> >> > > > > @pytest.mark.skip(reason="demonstrating skipping"))
> >> > > > > def test_foo(self)
> >> > > > >
> >> > > > > Which solution will be better for us?
> >> > > > >
> >> > > > > >
> >> > > > > > On Wed, Jan 1, 2020 at 8:14 PM Kamil BreguĆa <
> >> > > > kamil.bregula@polidea.com>
> >> > > > > > wrote:
> >> > > > > >
> >> > > > > > > > 1) Do we want to use additional plugins?
> >> > > > > > >
> >> > > > > > > Yes. We should use the full-power of plugins now.
> >> > > > > > >
> >> > > > > > > > 2) Do we want to use custom markers?
> >> > > > > > >
> >> > > > > > > Reply in a separate thread.
> >> > > > > > >
> >> > > > > > > >3) Do we want to use new assert statement?
> >> > > > > > >
> >> > > > > > > Reply in a separate thread
> >> > > > > > >
> >> > > > > > > > 4) Do we want to remove the inheritance from
> >> unittest.TestCase?
> >> > > > > > >
> >> > > > > > > Yes. After dropping support for Airflow 2.0, if possible. I
> >> would
> >> > > > > prefer to
> >> > > > > > > focus on working on new features for Airflow 2.0.
> >> > > > > > >
> >> > > > > > > > 5) Do we want to use class-less tests?
> >> > > > > > >
> >> > > > > > > No. Classes allow easy grouping of tests. Even if a file with
> >> one
> >> > > > > class now
> >> > > > > > > exists, a new one may appear in the future.
> >> > > > > > >
> >> > > > > > > > 6) Do we want to use pytest function instead of current?
> >> > > > > > >
> >> > > > > > > I feel good about the current functions. However, this is not
> >> a
> >> > > > serious
> >> > > > > > > relationship and I can create a new friendship.
> >> > > > > > >
> >> > > > > > > > 7) Do we want to use monkeypatch fixture?
> >> > > > > > >
> >> > > > > > > No. I prefer unittest.mock
> >> > > > > > >
> >> > > > > > > > 8) Do we want to use the pytest fixtures?
> >> > > > > > >
> >> > > > > > > No. I prefer classic fixtures, if possible. Their syntax is
> >> much
> >> > > > > simpler
> >> > > > > > > and easier to understand.
> >> > > > > > >
> >> > > > > > > On Wed, Jan 1, 2020 at 8:10 PM Kamil BreguĆa <
> >> > > > > kamil.bregula@polidea.com>
> >> > > > > > > wrote:
> >> > > > > > >
> >> > > > > > > > Hello,
> >> > > > > > > >
> >> > > > > > > >
> >> > > > > > > > We have recently migrated to pytest. Code written
> >> according to
> >> > > the
> >> > > > > > > > standard library - unittest.TestCase is still compatible
> >> with
> >> > > > pytest.
> >> > > > > > > >
> >> > > > > > > >
> >> > > > > > > > The AIP-21 document did not discuss the migration of
> >> current code
> >> > > > to
> >> > > > > > > > new features, but only discussed the change of runner and
> >> > > benefits
> >> > > > of
> >> > > > > > > > pytest over nosetets.
> >> > > > > > > >
> >> > > > > > > > Link:
> >> > > > > > > >
> >> > > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-27+Migrate+to+pytest
> >> > > > > > > >
> >> > > > > > > >
> >> > > > > > > > As far as I appreciate the many advantages of using this
> >> tool,
> >> > > but
> >> > > > I
> >> > > > > am
> >> > > > > > > > not sure **whether, how or when we want to start using some
> >> > > > > features**. I
> >> > > > > > > > prefer to keep the current project conventions in many
> >> areas,
> >> > > but I
> >> > > > > still
> >> > > > > > > > love pytest. I think we should establish general
> >> conventions on
> >> > > > > writing
> >> > > > > > > > tests and recommended solutions to known problems. I prefer
> >> a
> >> > > > > pragmatic
> >> > > > > > > > approach, not just the use of features, because now we can
> >> use
> >> > > it.
> >> > > > > > > > Unfortunately, I do not see many benefits from new features.
> >> > > > > > > >
> >> > > > > > > >
> >> > > > > > > > I would not like the code to have many ways of expressing
> >> the
> >> > > same
> >> > > > > > > > solution. For the following reasons:
> >> > > > > > > >
> >> > > > > > > >
> >> > > > > > > > * it can introduce a lack of understanding among new
> >> contributors
> >> > > > > > > >
> >> > > > > > > > * facilitate the understanding and reading of code.
> >> > > > > > > >
> >> > > > > > > > * creates unnecessary discussion about the preferences of
> >> one way
> >> > > > > over
> >> > > > > > > the
> >> > > > > > > > other. Not related to changes.
> >> > > > > > > >
> >> > > > > > > > * forces an understanding of the complex syntax of some
> >> > > solutions.
> >> > > > > > > >
> >> > > > > > > > * encourages people to rewrite the code, which can generate
> >> > > > > unnecessary
> >> > > > > > > > work.
> >> > > > > > > >
> >> > > > > > > >
> >> > > > > > > > To establish a full convention, I have prepared a few
> >> questions:
> >> > > > > > > >
> >> > > > > > > > 1) Do we want to use additional plugins when there is no
> >> function
> >> > > > in
> >> > > > > the
> >> > > > > > > > standard library e.g. flaky marker, forked marker? This
> >> is, in
> >> > > my
> >> > > > > > > opinion,
> >> > > > > > > > one of the big advantages of migrating to pytest.
> >> > > > > > > >
> >> > > > > > > > 2) Do we want to use custom markers?
> >> > > > > > > >
> >> > > > > > > > The discussion takes place in a separate thread:
> >> > > > > > > >
> >> > > > > > > >
> >> > > > > > > >
> >> > > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> https://lists.apache.org/thread.html/4538437c96f599766005ba7829d0bee1511debb4f53599e0d300a56f%40%3Cdev.airflow.apache.org%3E
> >> > > > > > > >
> >> > > > > > > > 3) Do we want to use new assert statement?
> >> > > > > > > >
> >> > > > > > > >
> >> > > > > > > >
> >> > > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> https://lists.apache.org/thread.html/08b64d3b084c865399f98f6c6f56235ce5329e843d97938e1a8045a5%40%3Cdev.airflow.apache.org%3E
> >> > > > > > > >
> >> > > > > > > > Based on the discussion with devlist, we want to delay
> >> migrations
> >> > > > to
> >> > > > > the
> >> > > > > > > > new assert statement.
> >> > > > > > > >
> >> > > > > > > > 4) Do we want to remove inheritance from unittest.TestCase?
> >> > > > > > > >
> >> > > > > > > > 5) Do we want to use class-less tests?
> >> > > > > > > >
> >> > > > > > > > 6) Do we want to use pytest function instead of current?
> >> > > > > > > >
> >> > > > > > > > For example:
> >> > > > > > > >
> >> > > > > > > > Unittest.assertRaises vs pytest.raises
> >> > > > > > > >
> >> > > > > > > > parametrize vs pytest.mark.parametrize
> >> > > > > > > >
> >> > > > > > > > unittest.skip[If], vs pytest.mark.skip[If]
> >> > > > > > > >
> >> > > > > > > > 7) Do we want to use monkeypatch fixture?
> >> > > > > > > >
> >> > > > > > > > https://docs.pytest.org/en/latest/monkeypatch.html
> >> > > > > > > >
> >> > > > > > > > 8) Do we want to use the pytest fixtures?
> >> > > > > > > >
> >> > > > > > > > Do we want to migrate all code to pytest fixtures?
> >> > > > > > > >
> >> > > > > > > > We are currently using a different style of fixtures. Do we
> >> want
> >> > > to
> >> > > > > give
> >> > > > > > > > it up?
> >> > > > > > > >
> >> > > > > > > >
> >> > > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> https://docs.python.org/3/library/unittest.html#class-and-module-fixtures
> >> > > > > > > >
> >> > > > > > > >
> >> > > > > > > > It is worth asking to think about whether we do not want to
> >> > > change
> >> > > > > this
> >> > > > > > > > convention in the future e.g. after dropping support for
> >> 1.10.X.
> >> > > We
> >> > > > > can
> >> > > > > > > > also allow two conventions on a temporary basis, and then
> >> migrate
> >> > > > to
> >> > > > > one
> >> > > > > > > at
> >> > > > > > > > a later time. For example, we want to migrate to the assert
> >> > > > statement
> >> > > > > > > after
> >> > > > > > > > dropping support for 1.10
> >> > > > > > > >
> >> > > > > > > >
> >> > > > > > > > I hope I found the main differences between the current
> >> > > convention
> >> > > > > and
> >> > > > > > > the
> >> > > > > > > > new convention. However, if you missed something, please
> >> continue
> >> > > > to
> >> > > > > > > number.
> >> > > > > > > >
> >> > > > > > > >
> >> > > > > > > > Best regards,
> >> > > > > > > >
> >> > > > > > > > Kamil
> >> > > > > > > >
> >> > > > > > > >
> >> > > > > > > >
> >> > > > > > > >
> >> > > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> > >
> >> > > --
> >> > >
> >> > > Jarek Potiuk
> >> > > Polidea <https://www.polidea.com/> | Principal Software Engineer
> >> > >
> >> > > M: +48 660 796 129 <+48660796129>
> >> > > [image: Polidea] <https://www.polidea.com/>
> >> > >
> >> >
> >> >
> >> > --
> >> >
> >> > Tomasz Urbaszek
> >> > Polidea <https://www.polidea.com/> | Software Engineer
> >> >
> >> > M: +48 505 628 493 <+48505628493>
> >> > E: tomasz.urbaszek@polidea.com <to...@polidea.com>
> >> >
> >> > Unique Tech
> >> > Check out our projects! <https://www.polidea.com/our-work>
> >>
> >
> >
> > --
> >
> > Jarek Potiuk
> > Polidea <https://www.polidea.com/> | Principal Software Engineer
> >
> > M: +48 660 796 129 <+48660796129>
> > [image: Polidea] <https://www.polidea.com/>
> >
> >
>
> --
>
> Jarek Potiuk
> Polidea <https://www.polidea.com/> | Principal Software Engineer
>
> M: +48 660 796 129 <+48660796129>
> [image: Polidea] <https://www.polidea.com/>
Re: Test style convention after migration to pytest
Posted by Jarek Potiuk <Ja...@polidea.com>.
Should we resume this :)?
On Mon, Jan 6, 2020 at 3:42 PM Jarek Potiuk <Ja...@polidea.com>
wrote:
> Good idea!
>
> On Mon, Jan 6, 2020 at 3:12 PM Kamil BreguĆa <ka...@polidea.com>
> wrote:
>
>> Hello,
>>
>> Now is the holiday period. Some people have not started working yet.
>> Others are busy with New Year activities. What do you think about
>> Friday?
>>
>> Best regards,
>> Kamil
>>
>> On Mon, Jan 6, 2020 at 2:48 PM Tomasz Urbaszek
>> <to...@polidea.com> wrote:
>> >
>> > When should we assume that we've reached a consensus?
>> >
>> > T.
>> >
>> > On Thu, Jan 2, 2020 at 12:52 AM Jarek Potiuk <Ja...@polidea.com>
>> > wrote:
>> >
>> > > > 1) Do we want to use additional plugins? *Yes*
>> > >
>> > > 2) Do we want to use custom markers? *Yes. They will help with
>> optimising
>> > > > our test execution.*
>> > >
>> > > 3) Do we want to use new assert statement? *Yes*
>> > > > 4) Do we want to remove the inheritance from unittest.TestCase? *No
>> > > > opinion about it. I am ok with both.*
>> > > > 5) Do we want to use class-less tests? *No.*
>> > > > 6) Do we want to use pytest function instead of current? *I don't
>> > > > understand. Can you explain please?*
>> > > > 7) Do we want to use monkeypatch fixture? *No. Mock is better.*
>> > > > 8) Do we want to use the pytest fixtures? *Yes.*
>> > > >
>> > >
>> > > Great that we have this discussion now. I also think we should just
>> agree
>> > > it now and not introduce it "globally".
>> > > Once we do it, we should simply add it together new features we
>> implement.
>> > > We have still pylint to finish as a "non-functional global change"
>> and we
>> > > should not add new one. It's good to continually improve but one
>> thing at a
>> > > time.
>> > >
>> > > BTW Pylint goes well we are down to 243 non-pylint files from 991
>> since
>> > > May.
>> > >
>> > > J.
>> > >
>> > > On Thu, Jan 2, 2020 at 12:40 AM Kaxil Naik <ka...@gmail.com>
>> wrote:
>> > >
>> > > > This is a tough one. Both arguments are reasonable.
>> > > >
>> > > > I agree at some point we should convert all to use assert. But at
>> the
>> > > same
>> > > > time, we should also focus on adding *more user-facing features *and
>> > > spend
>> > > > less time on more refactor or similar changes.
>> > > >
>> > > > So based on that, this might be a low priority. We also need to
>> still
>> > > > complete AIP-21
>> > > > <
>> > > >
>> > >
>> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-21%3A+Changes+in+import+paths
>> > > > >
>> > > > which is very critical for 2.0.
>> > > >
>> > > > 1) Do we want to use additional plugins?
>> > > >
>> > > >
>> > > > Yes.
>> > > >
>> > > > 2) Do we want to use custom markers?
>> > > >
>> > > >
>> > > > Not sure yet. Low Priority for me.
>> > > >
>> > > > 3) Do we want to use new assert statement?
>> > > >
>> > > >
>> > > > I think new PRs can contain it, shouldn't be a problem as long as
>> it is
>> > > > documented to avoid confusion.
>> > > >
>> > > > 4) Do we want to remove the inheritance from unittest.TestCase?
>> > > >
>> > > >
>> > > > Yes, this is, however, going to change how people write tests. So
>> someone
>> > > > has to own it as it can become painful with PRs getting merged
>> > > > continuously.
>> > > >
>> > > > 5) Do we want to use class-less tests?
>> > > >
>> > > >
>> > > > No.
>> > > >
>> > > > 6) Do we want to use pytest function instead of current?
>> > > >
>> > > >
>> > > > Yes
>> > > >
>> > > > 7) Do we want to use monkeypatch fixture?
>> > > >
>> > > >
>> > > > I also prefer unittest.mock but open to suggestions.
>> > > >
>> https://github.com/pytest-dev/pytest/issues/4576#issuecomment-449864333
>> > > > has
>> > > > some good comparison on it
>> > > >
>> > > > 8) Do we want to use the pytest fixtures?
>> > > >
>> > > >
>> > > > Yes
>> > > >
>> > > > On Wed, Jan 1, 2020 at 8:07 PM Kamil BreguĆa <
>> kamil.bregula@polidea.com>
>> > > > wrote:
>> > > >
>> > > > > @unittest.skip("demonstrating skipping")
>> > > > >
>> > > > > On Wed, Jan 1, 2020 at 8:37 PM Tomasz Urbaszek <
>> turbaszek@apache.org>
>> > > > > wrote:
>> > > > > >
>> > > > > >
>> > > > > > > 6) Do we want to use pytest function instead of current?
>> > > > > >
>> > > > > > I do not understand this point, can you explain?
>> > > > > >
>> > > > >
>> > > > > Pytest Introduces solutions that replace solutions that are now
>> used
>> > > > >
>> > > > > For example:
>> > > > > def test_foo(self):
>> > > > > wtih self.assertRaises(AirflowException):
>> > > > > bar()
>> > > > >
>> > > > > Can be replaced by following code:
>> > > > >
>> > > > > from pytest import raises
>> > > > >
>> > > > > wtih raises(AirflowException):
>> > > > > bar()
>> > > > >
>> > > > > OR
>> > > > >
>> > > > > from parametrize import parametrize
>> > > > >
>> > > > > @parametrize.expand([
>> > > > > (1, 1, ),
>> > > > > (2, 2, ),
>> > > > > ])
>> > > > > def test_foo(self, param_a, param_b);
>> > > > > self.assertEqual(param_a, param_b)
>> > > > >
>> > > > > can be replaced by
>> > > > >
>> > > > > @pytest.mark.parametrize("param_a,param_b", [(1, 1), (2, 2),])
>> > > > > def test_eval(param_a, param_b):
>> > > > > assert param_a == param_b
>> > > > >
>> > > > > OR
>> > > > >
>> > > > > @unittest.skip("demonstrating skipping")
>> > > > > def test_foo(self)
>> > > > >
>> > > > > can be replaced by
>> > > > >
>> > > > > @pytest.mark.skip(reason="demonstrating skipping"))
>> > > > > def test_foo(self)
>> > > > >
>> > > > > Which solution will be better for us?
>> > > > >
>> > > > > >
>> > > > > > On Wed, Jan 1, 2020 at 8:14 PM Kamil BreguĆa <
>> > > > kamil.bregula@polidea.com>
>> > > > > > wrote:
>> > > > > >
>> > > > > > > > 1) Do we want to use additional plugins?
>> > > > > > >
>> > > > > > > Yes. We should use the full-power of plugins now.
>> > > > > > >
>> > > > > > > > 2) Do we want to use custom markers?
>> > > > > > >
>> > > > > > > Reply in a separate thread.
>> > > > > > >
>> > > > > > > >3) Do we want to use new assert statement?
>> > > > > > >
>> > > > > > > Reply in a separate thread
>> > > > > > >
>> > > > > > > > 4) Do we want to remove the inheritance from
>> unittest.TestCase?
>> > > > > > >
>> > > > > > > Yes. After dropping support for Airflow 2.0, if possible. I
>> would
>> > > > > prefer to
>> > > > > > > focus on working on new features for Airflow 2.0.
>> > > > > > >
>> > > > > > > > 5) Do we want to use class-less tests?
>> > > > > > >
>> > > > > > > No. Classes allow easy grouping of tests. Even if a file with
>> one
>> > > > > class now
>> > > > > > > exists, a new one may appear in the future.
>> > > > > > >
>> > > > > > > > 6) Do we want to use pytest function instead of current?
>> > > > > > >
>> > > > > > > I feel good about the current functions. However, this is not
>> a
>> > > > serious
>> > > > > > > relationship and I can create a new friendship.
>> > > > > > >
>> > > > > > > > 7) Do we want to use monkeypatch fixture?
>> > > > > > >
>> > > > > > > No. I prefer unittest.mock
>> > > > > > >
>> > > > > > > > 8) Do we want to use the pytest fixtures?
>> > > > > > >
>> > > > > > > No. I prefer classic fixtures, if possible. Their syntax is
>> much
>> > > > > simpler
>> > > > > > > and easier to understand.
>> > > > > > >
>> > > > > > > On Wed, Jan 1, 2020 at 8:10 PM Kamil BreguĆa <
>> > > > > kamil.bregula@polidea.com>
>> > > > > > > wrote:
>> > > > > > >
>> > > > > > > > Hello,
>> > > > > > > >
>> > > > > > > >
>> > > > > > > > We have recently migrated to pytest. Code written
>> according to
>> > > the
>> > > > > > > > standard library - unittest.TestCase is still compatible
>> with
>> > > > pytest.
>> > > > > > > >
>> > > > > > > >
>> > > > > > > > The AIP-21 document did not discuss the migration of
>> current code
>> > > > to
>> > > > > > > > new features, but only discussed the change of runner and
>> > > benefits
>> > > > of
>> > > > > > > > pytest over nosetets.
>> > > > > > > >
>> > > > > > > > Link:
>> > > > > > > >
>> > > > > > >
>> > > > >
>> > > >
>> > >
>> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-27+Migrate+to+pytest
>> > > > > > > >
>> > > > > > > >
>> > > > > > > > As far as I appreciate the many advantages of using this
>> tool,
>> > > but
>> > > > I
>> > > > > am
>> > > > > > > > not sure **whether, how or when we want to start using some
>> > > > > features**. I
>> > > > > > > > prefer to keep the current project conventions in many
>> areas,
>> > > but I
>> > > > > still
>> > > > > > > > love pytest. I think we should establish general
>> conventions on
>> > > > > writing
>> > > > > > > > tests and recommended solutions to known problems. I prefer
>> a
>> > > > > pragmatic
>> > > > > > > > approach, not just the use of features, because now we can
>> use
>> > > it.
>> > > > > > > > Unfortunately, I do not see many benefits from new features.
>> > > > > > > >
>> > > > > > > >
>> > > > > > > > I would not like the code to have many ways of expressing
>> the
>> > > same
>> > > > > > > > solution. For the following reasons:
>> > > > > > > >
>> > > > > > > >
>> > > > > > > > * it can introduce a lack of understanding among new
>> contributors
>> > > > > > > >
>> > > > > > > > * facilitate the understanding and reading of code.
>> > > > > > > >
>> > > > > > > > * creates unnecessary discussion about the preferences of
>> one way
>> > > > > over
>> > > > > > > the
>> > > > > > > > other. Not related to changes.
>> > > > > > > >
>> > > > > > > > * forces an understanding of the complex syntax of some
>> > > solutions.
>> > > > > > > >
>> > > > > > > > * encourages people to rewrite the code, which can generate
>> > > > > unnecessary
>> > > > > > > > work.
>> > > > > > > >
>> > > > > > > >
>> > > > > > > > To establish a full convention, I have prepared a few
>> questions:
>> > > > > > > >
>> > > > > > > > 1) Do we want to use additional plugins when there is no
>> function
>> > > > in
>> > > > > the
>> > > > > > > > standard library e.g. flaky marker, forked marker? This
>> is, in
>> > > my
>> > > > > > > opinion,
>> > > > > > > > one of the big advantages of migrating to pytest.
>> > > > > > > >
>> > > > > > > > 2) Do we want to use custom markers?
>> > > > > > > >
>> > > > > > > > The discussion takes place in a separate thread:
>> > > > > > > >
>> > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > >
>> > > >
>> > >
>> https://lists.apache.org/thread.html/4538437c96f599766005ba7829d0bee1511debb4f53599e0d300a56f%40%3Cdev.airflow.apache.org%3E
>> > > > > > > >
>> > > > > > > > 3) Do we want to use new assert statement?
>> > > > > > > >
>> > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > >
>> > > >
>> > >
>> https://lists.apache.org/thread.html/08b64d3b084c865399f98f6c6f56235ce5329e843d97938e1a8045a5%40%3Cdev.airflow.apache.org%3E
>> > > > > > > >
>> > > > > > > > Based on the discussion with devlist, we want to delay
>> migrations
>> > > > to
>> > > > > the
>> > > > > > > > new assert statement.
>> > > > > > > >
>> > > > > > > > 4) Do we want to remove inheritance from unittest.TestCase?
>> > > > > > > >
>> > > > > > > > 5) Do we want to use class-less tests?
>> > > > > > > >
>> > > > > > > > 6) Do we want to use pytest function instead of current?
>> > > > > > > >
>> > > > > > > > For example:
>> > > > > > > >
>> > > > > > > > Unittest.assertRaises vs pytest.raises
>> > > > > > > >
>> > > > > > > > parametrize vs pytest.mark.parametrize
>> > > > > > > >
>> > > > > > > > unittest.skip[If], vs pytest.mark.skip[If]
>> > > > > > > >
>> > > > > > > > 7) Do we want to use monkeypatch fixture?
>> > > > > > > >
>> > > > > > > > https://docs.pytest.org/en/latest/monkeypatch.html
>> > > > > > > >
>> > > > > > > > 8) Do we want to use the pytest fixtures?
>> > > > > > > >
>> > > > > > > > Do we want to migrate all code to pytest fixtures?
>> > > > > > > >
>> > > > > > > > We are currently using a different style of fixtures. Do we
>> want
>> > > to
>> > > > > give
>> > > > > > > > it up?
>> > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > >
>> > > >
>> > >
>> https://docs.python.org/3/library/unittest.html#class-and-module-fixtures
>> > > > > > > >
>> > > > > > > >
>> > > > > > > > It is worth asking to think about whether we do not want to
>> > > change
>> > > > > this
>> > > > > > > > convention in the future e.g. after dropping support for
>> 1.10.X.
>> > > We
>> > > > > can
>> > > > > > > > also allow two conventions on a temporary basis, and then
>> migrate
>> > > > to
>> > > > > one
>> > > > > > > at
>> > > > > > > > a later time. For example, we want to migrate to the assert
>> > > > statement
>> > > > > > > after
>> > > > > > > > dropping support for 1.10
>> > > > > > > >
>> > > > > > > >
>> > > > > > > > I hope I found the main differences between the current
>> > > convention
>> > > > > and
>> > > > > > > the
>> > > > > > > > new convention. However, if you missed something, please
>> continue
>> > > > to
>> > > > > > > number.
>> > > > > > > >
>> > > > > > > >
>> > > > > > > > Best regards,
>> > > > > > > >
>> > > > > > > > Kamil
>> > > > > > > >
>> > > > > > > >
>> > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > >
>> > > >
>> > >
>> > >
>> > > --
>> > >
>> > > Jarek Potiuk
>> > > Polidea <https://www.polidea.com/> | Principal Software Engineer
>> > >
>> > > M: +48 660 796 129 <+48660796129>
>> > > [image: Polidea] <https://www.polidea.com/>
>> > >
>> >
>> >
>> > --
>> >
>> > Tomasz Urbaszek
>> > Polidea <https://www.polidea.com/> | Software Engineer
>> >
>> > M: +48 505 628 493 <+48505628493>
>> > E: tomasz.urbaszek@polidea.com <to...@polidea.com>
>> >
>> > Unique Tech
>> > Check out our projects! <https://www.polidea.com/our-work>
>>
>
>
> --
>
> Jarek Potiuk
> Polidea <https://www.polidea.com/> | Principal Software Engineer
>
> M: +48 660 796 129 <+48660796129>
> [image: Polidea] <https://www.polidea.com/>
>
>
--
Jarek Potiuk
Polidea <https://www.polidea.com/> | Principal Software Engineer
M: +48 660 796 129 <+48660796129>
[image: Polidea] <https://www.polidea.com/>
Re: Test style convention after migration to pytest
Posted by Jarek Potiuk <Ja...@polidea.com>.
Good idea!
On Mon, Jan 6, 2020 at 3:12 PM Kamil BreguĆa <ka...@polidea.com>
wrote:
> Hello,
>
> Now is the holiday period. Some people have not started working yet.
> Others are busy with New Year activities. What do you think about
> Friday?
>
> Best regards,
> Kamil
>
> On Mon, Jan 6, 2020 at 2:48 PM Tomasz Urbaszek
> <to...@polidea.com> wrote:
> >
> > When should we assume that we've reached a consensus?
> >
> > T.
> >
> > On Thu, Jan 2, 2020 at 12:52 AM Jarek Potiuk <Ja...@polidea.com>
> > wrote:
> >
> > > > 1) Do we want to use additional plugins? *Yes*
> > >
> > > 2) Do we want to use custom markers? *Yes. They will help with
> optimising
> > > > our test execution.*
> > >
> > > 3) Do we want to use new assert statement? *Yes*
> > > > 4) Do we want to remove the inheritance from unittest.TestCase? *No
> > > > opinion about it. I am ok with both.*
> > > > 5) Do we want to use class-less tests? *No.*
> > > > 6) Do we want to use pytest function instead of current? *I don't
> > > > understand. Can you explain please?*
> > > > 7) Do we want to use monkeypatch fixture? *No. Mock is better.*
> > > > 8) Do we want to use the pytest fixtures? *Yes.*
> > > >
> > >
> > > Great that we have this discussion now. I also think we should just
> agree
> > > it now and not introduce it "globally".
> > > Once we do it, we should simply add it together new features we
> implement.
> > > We have still pylint to finish as a "non-functional global change" and
> we
> > > should not add new one. It's good to continually improve but one thing
> at a
> > > time.
> > >
> > > BTW Pylint goes well we are down to 243 non-pylint files from 991 since
> > > May.
> > >
> > > J.
> > >
> > > On Thu, Jan 2, 2020 at 12:40 AM Kaxil Naik <ka...@gmail.com>
> wrote:
> > >
> > > > This is a tough one. Both arguments are reasonable.
> > > >
> > > > I agree at some point we should convert all to use assert. But at the
> > > same
> > > > time, we should also focus on adding *more user-facing features *and
> > > spend
> > > > less time on more refactor or similar changes.
> > > >
> > > > So based on that, this might be a low priority. We also need to still
> > > > complete AIP-21
> > > > <
> > > >
> > >
> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-21%3A+Changes+in+import+paths
> > > > >
> > > > which is very critical for 2.0.
> > > >
> > > > 1) Do we want to use additional plugins?
> > > >
> > > >
> > > > Yes.
> > > >
> > > > 2) Do we want to use custom markers?
> > > >
> > > >
> > > > Not sure yet. Low Priority for me.
> > > >
> > > > 3) Do we want to use new assert statement?
> > > >
> > > >
> > > > I think new PRs can contain it, shouldn't be a problem as long as it
> is
> > > > documented to avoid confusion.
> > > >
> > > > 4) Do we want to remove the inheritance from unittest.TestCase?
> > > >
> > > >
> > > > Yes, this is, however, going to change how people write tests. So
> someone
> > > > has to own it as it can become painful with PRs getting merged
> > > > continuously.
> > > >
> > > > 5) Do we want to use class-less tests?
> > > >
> > > >
> > > > No.
> > > >
> > > > 6) Do we want to use pytest function instead of current?
> > > >
> > > >
> > > > Yes
> > > >
> > > > 7) Do we want to use monkeypatch fixture?
> > > >
> > > >
> > > > I also prefer unittest.mock but open to suggestions.
> > > >
> https://github.com/pytest-dev/pytest/issues/4576#issuecomment-449864333
> > > > has
> > > > some good comparison on it
> > > >
> > > > 8) Do we want to use the pytest fixtures?
> > > >
> > > >
> > > > Yes
> > > >
> > > > On Wed, Jan 1, 2020 at 8:07 PM Kamil BreguĆa <
> kamil.bregula@polidea.com>
> > > > wrote:
> > > >
> > > > > @unittest.skip("demonstrating skipping")
> > > > >
> > > > > On Wed, Jan 1, 2020 at 8:37 PM Tomasz Urbaszek <
> turbaszek@apache.org>
> > > > > wrote:
> > > > > >
> > > > > >
> > > > > > > 6) Do we want to use pytest function instead of current?
> > > > > >
> > > > > > I do not understand this point, can you explain?
> > > > > >
> > > > >
> > > > > Pytest Introduces solutions that replace solutions that are now
> used
> > > > >
> > > > > For example:
> > > > > def test_foo(self):
> > > > > wtih self.assertRaises(AirflowException):
> > > > > bar()
> > > > >
> > > > > Can be replaced by following code:
> > > > >
> > > > > from pytest import raises
> > > > >
> > > > > wtih raises(AirflowException):
> > > > > bar()
> > > > >
> > > > > OR
> > > > >
> > > > > from parametrize import parametrize
> > > > >
> > > > > @parametrize.expand([
> > > > > (1, 1, ),
> > > > > (2, 2, ),
> > > > > ])
> > > > > def test_foo(self, param_a, param_b);
> > > > > self.assertEqual(param_a, param_b)
> > > > >
> > > > > can be replaced by
> > > > >
> > > > > @pytest.mark.parametrize("param_a,param_b", [(1, 1), (2, 2),])
> > > > > def test_eval(param_a, param_b):
> > > > > assert param_a == param_b
> > > > >
> > > > > OR
> > > > >
> > > > > @unittest.skip("demonstrating skipping")
> > > > > def test_foo(self)
> > > > >
> > > > > can be replaced by
> > > > >
> > > > > @pytest.mark.skip(reason="demonstrating skipping"))
> > > > > def test_foo(self)
> > > > >
> > > > > Which solution will be better for us?
> > > > >
> > > > > >
> > > > > > On Wed, Jan 1, 2020 at 8:14 PM Kamil BreguĆa <
> > > > kamil.bregula@polidea.com>
> > > > > > wrote:
> > > > > >
> > > > > > > > 1) Do we want to use additional plugins?
> > > > > > >
> > > > > > > Yes. We should use the full-power of plugins now.
> > > > > > >
> > > > > > > > 2) Do we want to use custom markers?
> > > > > > >
> > > > > > > Reply in a separate thread.
> > > > > > >
> > > > > > > >3) Do we want to use new assert statement?
> > > > > > >
> > > > > > > Reply in a separate thread
> > > > > > >
> > > > > > > > 4) Do we want to remove the inheritance from
> unittest.TestCase?
> > > > > > >
> > > > > > > Yes. After dropping support for Airflow 2.0, if possible. I
> would
> > > > > prefer to
> > > > > > > focus on working on new features for Airflow 2.0.
> > > > > > >
> > > > > > > > 5) Do we want to use class-less tests?
> > > > > > >
> > > > > > > No. Classes allow easy grouping of tests. Even if a file with
> one
> > > > > class now
> > > > > > > exists, a new one may appear in the future.
> > > > > > >
> > > > > > > > 6) Do we want to use pytest function instead of current?
> > > > > > >
> > > > > > > I feel good about the current functions. However, this is not a
> > > > serious
> > > > > > > relationship and I can create a new friendship.
> > > > > > >
> > > > > > > > 7) Do we want to use monkeypatch fixture?
> > > > > > >
> > > > > > > No. I prefer unittest.mock
> > > > > > >
> > > > > > > > 8) Do we want to use the pytest fixtures?
> > > > > > >
> > > > > > > No. I prefer classic fixtures, if possible. Their syntax is
> much
> > > > > simpler
> > > > > > > and easier to understand.
> > > > > > >
> > > > > > > On Wed, Jan 1, 2020 at 8:10 PM Kamil BreguĆa <
> > > > > kamil.bregula@polidea.com>
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Hello,
> > > > > > > >
> > > > > > > >
> > > > > > > > We have recently migrated to pytest. Code written according
> to
> > > the
> > > > > > > > standard library - unittest.TestCase is still compatible with
> > > > pytest.
> > > > > > > >
> > > > > > > >
> > > > > > > > The AIP-21 document did not discuss the migration of current
> code
> > > > to
> > > > > > > > new features, but only discussed the change of runner and
> > > benefits
> > > > of
> > > > > > > > pytest over nosetets.
> > > > > > > >
> > > > > > > > Link:
> > > > > > > >
> > > > > > >
> > > > >
> > > >
> > >
> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-27+Migrate+to+pytest
> > > > > > > >
> > > > > > > >
> > > > > > > > As far as I appreciate the many advantages of using this
> tool,
> > > but
> > > > I
> > > > > am
> > > > > > > > not sure **whether, how or when we want to start using some
> > > > > features**. I
> > > > > > > > prefer to keep the current project conventions in many areas,
> > > but I
> > > > > still
> > > > > > > > love pytest. I think we should establish general conventions
> on
> > > > > writing
> > > > > > > > tests and recommended solutions to known problems. I prefer a
> > > > > pragmatic
> > > > > > > > approach, not just the use of features, because now we can
> use
> > > it.
> > > > > > > > Unfortunately, I do not see many benefits from new features.
> > > > > > > >
> > > > > > > >
> > > > > > > > I would not like the code to have many ways of expressing the
> > > same
> > > > > > > > solution. For the following reasons:
> > > > > > > >
> > > > > > > >
> > > > > > > > * it can introduce a lack of understanding among new
> contributors
> > > > > > > >
> > > > > > > > * facilitate the understanding and reading of code.
> > > > > > > >
> > > > > > > > * creates unnecessary discussion about the preferences of
> one way
> > > > > over
> > > > > > > the
> > > > > > > > other. Not related to changes.
> > > > > > > >
> > > > > > > > * forces an understanding of the complex syntax of some
> > > solutions.
> > > > > > > >
> > > > > > > > * encourages people to rewrite the code, which can generate
> > > > > unnecessary
> > > > > > > > work.
> > > > > > > >
> > > > > > > >
> > > > > > > > To establish a full convention, I have prepared a few
> questions:
> > > > > > > >
> > > > > > > > 1) Do we want to use additional plugins when there is no
> function
> > > > in
> > > > > the
> > > > > > > > standard library e.g. flaky marker, forked marker? This is,
> in
> > > my
> > > > > > > opinion,
> > > > > > > > one of the big advantages of migrating to pytest.
> > > > > > > >
> > > > > > > > 2) Do we want to use custom markers?
> > > > > > > >
> > > > > > > > The discussion takes place in a separate thread:
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > >
> > > >
> > >
> https://lists.apache.org/thread.html/4538437c96f599766005ba7829d0bee1511debb4f53599e0d300a56f%40%3Cdev.airflow.apache.org%3E
> > > > > > > >
> > > > > > > > 3) Do we want to use new assert statement?
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > >
> > > >
> > >
> https://lists.apache.org/thread.html/08b64d3b084c865399f98f6c6f56235ce5329e843d97938e1a8045a5%40%3Cdev.airflow.apache.org%3E
> > > > > > > >
> > > > > > > > Based on the discussion with devlist, we want to delay
> migrations
> > > > to
> > > > > the
> > > > > > > > new assert statement.
> > > > > > > >
> > > > > > > > 4) Do we want to remove inheritance from unittest.TestCase?
> > > > > > > >
> > > > > > > > 5) Do we want to use class-less tests?
> > > > > > > >
> > > > > > > > 6) Do we want to use pytest function instead of current?
> > > > > > > >
> > > > > > > > For example:
> > > > > > > >
> > > > > > > > Unittest.assertRaises vs pytest.raises
> > > > > > > >
> > > > > > > > parametrize vs pytest.mark.parametrize
> > > > > > > >
> > > > > > > > unittest.skip[If], vs pytest.mark.skip[If]
> > > > > > > >
> > > > > > > > 7) Do we want to use monkeypatch fixture?
> > > > > > > >
> > > > > > > > https://docs.pytest.org/en/latest/monkeypatch.html
> > > > > > > >
> > > > > > > > 8) Do we want to use the pytest fixtures?
> > > > > > > >
> > > > > > > > Do we want to migrate all code to pytest fixtures?
> > > > > > > >
> > > > > > > > We are currently using a different style of fixtures. Do we
> want
> > > to
> > > > > give
> > > > > > > > it up?
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > >
> > > >
> > >
> https://docs.python.org/3/library/unittest.html#class-and-module-fixtures
> > > > > > > >
> > > > > > > >
> > > > > > > > It is worth asking to think about whether we do not want to
> > > change
> > > > > this
> > > > > > > > convention in the future e.g. after dropping support for
> 1.10.X.
> > > We
> > > > > can
> > > > > > > > also allow two conventions on a temporary basis, and then
> migrate
> > > > to
> > > > > one
> > > > > > > at
> > > > > > > > a later time. For example, we want to migrate to the assert
> > > > statement
> > > > > > > after
> > > > > > > > dropping support for 1.10
> > > > > > > >
> > > > > > > >
> > > > > > > > I hope I found the main differences between the current
> > > convention
> > > > > and
> > > > > > > the
> > > > > > > > new convention. However, if you missed something, please
> continue
> > > > to
> > > > > > > number.
> > > > > > > >
> > > > > > > >
> > > > > > > > Best regards,
> > > > > > > >
> > > > > > > > Kamil
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > >
> > > >
> > >
> > >
> > > --
> > >
> > > Jarek Potiuk
> > > Polidea <https://www.polidea.com/> | Principal Software Engineer
> > >
> > > M: +48 660 796 129 <+48660796129>
> > > [image: Polidea] <https://www.polidea.com/>
> > >
> >
> >
> > --
> >
> > Tomasz Urbaszek
> > Polidea <https://www.polidea.com/> | Software Engineer
> >
> > M: +48 505 628 493 <+48505628493>
> > E: tomasz.urbaszek@polidea.com <to...@polidea.com>
> >
> > Unique Tech
> > Check out our projects! <https://www.polidea.com/our-work>
>
--
Jarek Potiuk
Polidea <https://www.polidea.com/> | Principal Software Engineer
M: +48 660 796 129 <+48660796129>
[image: Polidea] <https://www.polidea.com/>
Re: Test style convention after migration to pytest
Posted by Kamil BreguĆa <ka...@polidea.com>.
Hello,
Now is the holiday period. Some people have not started working yet.
Others are busy with New Year activities. What do you think about
Friday?
Best regards,
Kamil
On Mon, Jan 6, 2020 at 2:48 PM Tomasz Urbaszek
<to...@polidea.com> wrote:
>
> When should we assume that we've reached a consensus?
>
> T.
>
> On Thu, Jan 2, 2020 at 12:52 AM Jarek Potiuk <Ja...@polidea.com>
> wrote:
>
> > > 1) Do we want to use additional plugins? *Yes*
> >
> > 2) Do we want to use custom markers? *Yes. They will help with optimising
> > > our test execution.*
> >
> > 3) Do we want to use new assert statement? *Yes*
> > > 4) Do we want to remove the inheritance from unittest.TestCase? *No
> > > opinion about it. I am ok with both.*
> > > 5) Do we want to use class-less tests? *No.*
> > > 6) Do we want to use pytest function instead of current? *I don't
> > > understand. Can you explain please?*
> > > 7) Do we want to use monkeypatch fixture? *No. Mock is better.*
> > > 8) Do we want to use the pytest fixtures? *Yes.*
> > >
> >
> > Great that we have this discussion now. I also think we should just agree
> > it now and not introduce it "globally".
> > Once we do it, we should simply add it together new features we implement.
> > We have still pylint to finish as a "non-functional global change" and we
> > should not add new one. It's good to continually improve but one thing at a
> > time.
> >
> > BTW Pylint goes well we are down to 243 non-pylint files from 991 since
> > May.
> >
> > J.
> >
> > On Thu, Jan 2, 2020 at 12:40 AM Kaxil Naik <ka...@gmail.com> wrote:
> >
> > > This is a tough one. Both arguments are reasonable.
> > >
> > > I agree at some point we should convert all to use assert. But at the
> > same
> > > time, we should also focus on adding *more user-facing features *and
> > spend
> > > less time on more refactor or similar changes.
> > >
> > > So based on that, this might be a low priority. We also need to still
> > > complete AIP-21
> > > <
> > >
> > https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-21%3A+Changes+in+import+paths
> > > >
> > > which is very critical for 2.0.
> > >
> > > 1) Do we want to use additional plugins?
> > >
> > >
> > > Yes.
> > >
> > > 2) Do we want to use custom markers?
> > >
> > >
> > > Not sure yet. Low Priority for me.
> > >
> > > 3) Do we want to use new assert statement?
> > >
> > >
> > > I think new PRs can contain it, shouldn't be a problem as long as it is
> > > documented to avoid confusion.
> > >
> > > 4) Do we want to remove the inheritance from unittest.TestCase?
> > >
> > >
> > > Yes, this is, however, going to change how people write tests. So someone
> > > has to own it as it can become painful with PRs getting merged
> > > continuously.
> > >
> > > 5) Do we want to use class-less tests?
> > >
> > >
> > > No.
> > >
> > > 6) Do we want to use pytest function instead of current?
> > >
> > >
> > > Yes
> > >
> > > 7) Do we want to use monkeypatch fixture?
> > >
> > >
> > > I also prefer unittest.mock but open to suggestions.
> > > https://github.com/pytest-dev/pytest/issues/4576#issuecomment-449864333
> > > has
> > > some good comparison on it
> > >
> > > 8) Do we want to use the pytest fixtures?
> > >
> > >
> > > Yes
> > >
> > > On Wed, Jan 1, 2020 at 8:07 PM Kamil BreguĆa <ka...@polidea.com>
> > > wrote:
> > >
> > > > @unittest.skip("demonstrating skipping")
> > > >
> > > > On Wed, Jan 1, 2020 at 8:37 PM Tomasz Urbaszek <tu...@apache.org>
> > > > wrote:
> > > > >
> > > > >
> > > > > > 6) Do we want to use pytest function instead of current?
> > > > >
> > > > > I do not understand this point, can you explain?
> > > > >
> > > >
> > > > Pytest Introduces solutions that replace solutions that are now used
> > > >
> > > > For example:
> > > > def test_foo(self):
> > > > wtih self.assertRaises(AirflowException):
> > > > bar()
> > > >
> > > > Can be replaced by following code:
> > > >
> > > > from pytest import raises
> > > >
> > > > wtih raises(AirflowException):
> > > > bar()
> > > >
> > > > OR
> > > >
> > > > from parametrize import parametrize
> > > >
> > > > @parametrize.expand([
> > > > (1, 1, ),
> > > > (2, 2, ),
> > > > ])
> > > > def test_foo(self, param_a, param_b);
> > > > self.assertEqual(param_a, param_b)
> > > >
> > > > can be replaced by
> > > >
> > > > @pytest.mark.parametrize("param_a,param_b", [(1, 1), (2, 2),])
> > > > def test_eval(param_a, param_b):
> > > > assert param_a == param_b
> > > >
> > > > OR
> > > >
> > > > @unittest.skip("demonstrating skipping")
> > > > def test_foo(self)
> > > >
> > > > can be replaced by
> > > >
> > > > @pytest.mark.skip(reason="demonstrating skipping"))
> > > > def test_foo(self)
> > > >
> > > > Which solution will be better for us?
> > > >
> > > > >
> > > > > On Wed, Jan 1, 2020 at 8:14 PM Kamil BreguĆa <
> > > kamil.bregula@polidea.com>
> > > > > wrote:
> > > > >
> > > > > > > 1) Do we want to use additional plugins?
> > > > > >
> > > > > > Yes. We should use the full-power of plugins now.
> > > > > >
> > > > > > > 2) Do we want to use custom markers?
> > > > > >
> > > > > > Reply in a separate thread.
> > > > > >
> > > > > > >3) Do we want to use new assert statement?
> > > > > >
> > > > > > Reply in a separate thread
> > > > > >
> > > > > > > 4) Do we want to remove the inheritance from unittest.TestCase?
> > > > > >
> > > > > > Yes. After dropping support for Airflow 2.0, if possible. I would
> > > > prefer to
> > > > > > focus on working on new features for Airflow 2.0.
> > > > > >
> > > > > > > 5) Do we want to use class-less tests?
> > > > > >
> > > > > > No. Classes allow easy grouping of tests. Even if a file with one
> > > > class now
> > > > > > exists, a new one may appear in the future.
> > > > > >
> > > > > > > 6) Do we want to use pytest function instead of current?
> > > > > >
> > > > > > I feel good about the current functions. However, this is not a
> > > serious
> > > > > > relationship and I can create a new friendship.
> > > > > >
> > > > > > > 7) Do we want to use monkeypatch fixture?
> > > > > >
> > > > > > No. I prefer unittest.mock
> > > > > >
> > > > > > > 8) Do we want to use the pytest fixtures?
> > > > > >
> > > > > > No. I prefer classic fixtures, if possible. Their syntax is much
> > > > simpler
> > > > > > and easier to understand.
> > > > > >
> > > > > > On Wed, Jan 1, 2020 at 8:10 PM Kamil BreguĆa <
> > > > kamil.bregula@polidea.com>
> > > > > > wrote:
> > > > > >
> > > > > > > Hello,
> > > > > > >
> > > > > > >
> > > > > > > We have recently migrated to pytest. Code written according to
> > the
> > > > > > > standard library - unittest.TestCase is still compatible with
> > > pytest.
> > > > > > >
> > > > > > >
> > > > > > > The AIP-21 document did not discuss the migration of current code
> > > to
> > > > > > > new features, but only discussed the change of runner and
> > benefits
> > > of
> > > > > > > pytest over nosetets.
> > > > > > >
> > > > > > > Link:
> > > > > > >
> > > > > >
> > > >
> > >
> > https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-27+Migrate+to+pytest
> > > > > > >
> > > > > > >
> > > > > > > As far as I appreciate the many advantages of using this tool,
> > but
> > > I
> > > > am
> > > > > > > not sure **whether, how or when we want to start using some
> > > > features**. I
> > > > > > > prefer to keep the current project conventions in many areas,
> > but I
> > > > still
> > > > > > > love pytest. I think we should establish general conventions on
> > > > writing
> > > > > > > tests and recommended solutions to known problems. I prefer a
> > > > pragmatic
> > > > > > > approach, not just the use of features, because now we can use
> > it.
> > > > > > > Unfortunately, I do not see many benefits from new features.
> > > > > > >
> > > > > > >
> > > > > > > I would not like the code to have many ways of expressing the
> > same
> > > > > > > solution. For the following reasons:
> > > > > > >
> > > > > > >
> > > > > > > * it can introduce a lack of understanding among new contributors
> > > > > > >
> > > > > > > * facilitate the understanding and reading of code.
> > > > > > >
> > > > > > > * creates unnecessary discussion about the preferences of one way
> > > > over
> > > > > > the
> > > > > > > other. Not related to changes.
> > > > > > >
> > > > > > > * forces an understanding of the complex syntax of some
> > solutions.
> > > > > > >
> > > > > > > * encourages people to rewrite the code, which can generate
> > > > unnecessary
> > > > > > > work.
> > > > > > >
> > > > > > >
> > > > > > > To establish a full convention, I have prepared a few questions:
> > > > > > >
> > > > > > > 1) Do we want to use additional plugins when there is no function
> > > in
> > > > the
> > > > > > > standard library e.g. flaky marker, forked marker? This is, in
> > my
> > > > > > opinion,
> > > > > > > one of the big advantages of migrating to pytest.
> > > > > > >
> > > > > > > 2) Do we want to use custom markers?
> > > > > > >
> > > > > > > The discussion takes place in a separate thread:
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > >
> > > >
> > >
> > https://lists.apache.org/thread.html/4538437c96f599766005ba7829d0bee1511debb4f53599e0d300a56f%40%3Cdev.airflow.apache.org%3E
> > > > > > >
> > > > > > > 3) Do we want to use new assert statement?
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > >
> > > >
> > >
> > https://lists.apache.org/thread.html/08b64d3b084c865399f98f6c6f56235ce5329e843d97938e1a8045a5%40%3Cdev.airflow.apache.org%3E
> > > > > > >
> > > > > > > Based on the discussion with devlist, we want to delay migrations
> > > to
> > > > the
> > > > > > > new assert statement.
> > > > > > >
> > > > > > > 4) Do we want to remove inheritance from unittest.TestCase?
> > > > > > >
> > > > > > > 5) Do we want to use class-less tests?
> > > > > > >
> > > > > > > 6) Do we want to use pytest function instead of current?
> > > > > > >
> > > > > > > For example:
> > > > > > >
> > > > > > > Unittest.assertRaises vs pytest.raises
> > > > > > >
> > > > > > > parametrize vs pytest.mark.parametrize
> > > > > > >
> > > > > > > unittest.skip[If], vs pytest.mark.skip[If]
> > > > > > >
> > > > > > > 7) Do we want to use monkeypatch fixture?
> > > > > > >
> > > > > > > https://docs.pytest.org/en/latest/monkeypatch.html
> > > > > > >
> > > > > > > 8) Do we want to use the pytest fixtures?
> > > > > > >
> > > > > > > Do we want to migrate all code to pytest fixtures?
> > > > > > >
> > > > > > > We are currently using a different style of fixtures. Do we want
> > to
> > > > give
> > > > > > > it up?
> > > > > > >
> > > > > > >
> > > > > >
> > > >
> > >
> > https://docs.python.org/3/library/unittest.html#class-and-module-fixtures
> > > > > > >
> > > > > > >
> > > > > > > It is worth asking to think about whether we do not want to
> > change
> > > > this
> > > > > > > convention in the future e.g. after dropping support for 1.10.X.
> > We
> > > > can
> > > > > > > also allow two conventions on a temporary basis, and then migrate
> > > to
> > > > one
> > > > > > at
> > > > > > > a later time. For example, we want to migrate to the assert
> > > statement
> > > > > > after
> > > > > > > dropping support for 1.10
> > > > > > >
> > > > > > >
> > > > > > > I hope I found the main differences between the current
> > convention
> > > > and
> > > > > > the
> > > > > > > new convention. However, if you missed something, please continue
> > > to
> > > > > > number.
> > > > > > >
> > > > > > >
> > > > > > > Best regards,
> > > > > > >
> > > > > > > Kamil
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > >
> > > >
> > >
> >
> >
> > --
> >
> > Jarek Potiuk
> > Polidea <https://www.polidea.com/> | Principal Software Engineer
> >
> > M: +48 660 796 129 <+48660796129>
> > [image: Polidea] <https://www.polidea.com/>
> >
>
>
> --
>
> Tomasz Urbaszek
> Polidea <https://www.polidea.com/> | Software Engineer
>
> M: +48 505 628 493 <+48505628493>
> E: tomasz.urbaszek@polidea.com <to...@polidea.com>
>
> Unique Tech
> Check out our projects! <https://www.polidea.com/our-work>
Re: Test style convention after migration to pytest
Posted by Tomasz Urbaszek <to...@polidea.com>.
When should we assume that we've reached a consensus?
T.
On Thu, Jan 2, 2020 at 12:52 AM Jarek Potiuk <Ja...@polidea.com>
wrote:
> > 1) Do we want to use additional plugins? *Yes*
>
> 2) Do we want to use custom markers? *Yes. They will help with optimising
> > our test execution.*
>
> 3) Do we want to use new assert statement? *Yes*
> > 4) Do we want to remove the inheritance from unittest.TestCase? *No
> > opinion about it. I am ok with both.*
> > 5) Do we want to use class-less tests? *No.*
> > 6) Do we want to use pytest function instead of current? *I don't
> > understand. Can you explain please?*
> > 7) Do we want to use monkeypatch fixture? *No. Mock is better.*
> > 8) Do we want to use the pytest fixtures? *Yes.*
> >
>
> Great that we have this discussion now. I also think we should just agree
> it now and not introduce it "globally".
> Once we do it, we should simply add it together new features we implement.
> We have still pylint to finish as a "non-functional global change" and we
> should not add new one. It's good to continually improve but one thing at a
> time.
>
> BTW Pylint goes well we are down to 243 non-pylint files from 991 since
> May.
>
> J.
>
> On Thu, Jan 2, 2020 at 12:40 AM Kaxil Naik <ka...@gmail.com> wrote:
>
> > This is a tough one. Both arguments are reasonable.
> >
> > I agree at some point we should convert all to use assert. But at the
> same
> > time, we should also focus on adding *more user-facing features *and
> spend
> > less time on more refactor or similar changes.
> >
> > So based on that, this might be a low priority. We also need to still
> > complete AIP-21
> > <
> >
> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-21%3A+Changes+in+import+paths
> > >
> > which is very critical for 2.0.
> >
> > 1) Do we want to use additional plugins?
> >
> >
> > Yes.
> >
> > 2) Do we want to use custom markers?
> >
> >
> > Not sure yet. Low Priority for me.
> >
> > 3) Do we want to use new assert statement?
> >
> >
> > I think new PRs can contain it, shouldn't be a problem as long as it is
> > documented to avoid confusion.
> >
> > 4) Do we want to remove the inheritance from unittest.TestCase?
> >
> >
> > Yes, this is, however, going to change how people write tests. So someone
> > has to own it as it can become painful with PRs getting merged
> > continuously.
> >
> > 5) Do we want to use class-less tests?
> >
> >
> > No.
> >
> > 6) Do we want to use pytest function instead of current?
> >
> >
> > Yes
> >
> > 7) Do we want to use monkeypatch fixture?
> >
> >
> > I also prefer unittest.mock but open to suggestions.
> > https://github.com/pytest-dev/pytest/issues/4576#issuecomment-449864333
> > has
> > some good comparison on it
> >
> > 8) Do we want to use the pytest fixtures?
> >
> >
> > Yes
> >
> > On Wed, Jan 1, 2020 at 8:07 PM Kamil BreguĆa <ka...@polidea.com>
> > wrote:
> >
> > > @unittest.skip("demonstrating skipping")
> > >
> > > On Wed, Jan 1, 2020 at 8:37 PM Tomasz Urbaszek <tu...@apache.org>
> > > wrote:
> > > >
> > > >
> > > > > 6) Do we want to use pytest function instead of current?
> > > >
> > > > I do not understand this point, can you explain?
> > > >
> > >
> > > Pytest Introduces solutions that replace solutions that are now used
> > >
> > > For example:
> > > def test_foo(self):
> > > wtih self.assertRaises(AirflowException):
> > > bar()
> > >
> > > Can be replaced by following code:
> > >
> > > from pytest import raises
> > >
> > > wtih raises(AirflowException):
> > > bar()
> > >
> > > OR
> > >
> > > from parametrize import parametrize
> > >
> > > @parametrize.expand([
> > > (1, 1, ),
> > > (2, 2, ),
> > > ])
> > > def test_foo(self, param_a, param_b);
> > > self.assertEqual(param_a, param_b)
> > >
> > > can be replaced by
> > >
> > > @pytest.mark.parametrize("param_a,param_b", [(1, 1), (2, 2),])
> > > def test_eval(param_a, param_b):
> > > assert param_a == param_b
> > >
> > > OR
> > >
> > > @unittest.skip("demonstrating skipping")
> > > def test_foo(self)
> > >
> > > can be replaced by
> > >
> > > @pytest.mark.skip(reason="demonstrating skipping"))
> > > def test_foo(self)
> > >
> > > Which solution will be better for us?
> > >
> > > >
> > > > On Wed, Jan 1, 2020 at 8:14 PM Kamil BreguĆa <
> > kamil.bregula@polidea.com>
> > > > wrote:
> > > >
> > > > > > 1) Do we want to use additional plugins?
> > > > >
> > > > > Yes. We should use the full-power of plugins now.
> > > > >
> > > > > > 2) Do we want to use custom markers?
> > > > >
> > > > > Reply in a separate thread.
> > > > >
> > > > > >3) Do we want to use new assert statement?
> > > > >
> > > > > Reply in a separate thread
> > > > >
> > > > > > 4) Do we want to remove the inheritance from unittest.TestCase?
> > > > >
> > > > > Yes. After dropping support for Airflow 2.0, if possible. I would
> > > prefer to
> > > > > focus on working on new features for Airflow 2.0.
> > > > >
> > > > > > 5) Do we want to use class-less tests?
> > > > >
> > > > > No. Classes allow easy grouping of tests. Even if a file with one
> > > class now
> > > > > exists, a new one may appear in the future.
> > > > >
> > > > > > 6) Do we want to use pytest function instead of current?
> > > > >
> > > > > I feel good about the current functions. However, this is not a
> > serious
> > > > > relationship and I can create a new friendship.
> > > > >
> > > > > > 7) Do we want to use monkeypatch fixture?
> > > > >
> > > > > No. I prefer unittest.mock
> > > > >
> > > > > > 8) Do we want to use the pytest fixtures?
> > > > >
> > > > > No. I prefer classic fixtures, if possible. Their syntax is much
> > > simpler
> > > > > and easier to understand.
> > > > >
> > > > > On Wed, Jan 1, 2020 at 8:10 PM Kamil BreguĆa <
> > > kamil.bregula@polidea.com>
> > > > > wrote:
> > > > >
> > > > > > Hello,
> > > > > >
> > > > > >
> > > > > > We have recently migrated to pytest. Code written according to
> the
> > > > > > standard library - unittest.TestCase is still compatible with
> > pytest.
> > > > > >
> > > > > >
> > > > > > The AIP-21 document did not discuss the migration of current code
> > to
> > > > > > new features, but only discussed the change of runner and
> benefits
> > of
> > > > > > pytest over nosetets.
> > > > > >
> > > > > > Link:
> > > > > >
> > > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-27+Migrate+to+pytest
> > > > > >
> > > > > >
> > > > > > As far as I appreciate the many advantages of using this tool,
> but
> > I
> > > am
> > > > > > not sure **whether, how or when we want to start using some
> > > features**. I
> > > > > > prefer to keep the current project conventions in many areas,
> but I
> > > still
> > > > > > love pytest. I think we should establish general conventions on
> > > writing
> > > > > > tests and recommended solutions to known problems. I prefer a
> > > pragmatic
> > > > > > approach, not just the use of features, because now we can use
> it.
> > > > > > Unfortunately, I do not see many benefits from new features.
> > > > > >
> > > > > >
> > > > > > I would not like the code to have many ways of expressing the
> same
> > > > > > solution. For the following reasons:
> > > > > >
> > > > > >
> > > > > > * it can introduce a lack of understanding among new contributors
> > > > > >
> > > > > > * facilitate the understanding and reading of code.
> > > > > >
> > > > > > * creates unnecessary discussion about the preferences of one way
> > > over
> > > > > the
> > > > > > other. Not related to changes.
> > > > > >
> > > > > > * forces an understanding of the complex syntax of some
> solutions.
> > > > > >
> > > > > > * encourages people to rewrite the code, which can generate
> > > unnecessary
> > > > > > work.
> > > > > >
> > > > > >
> > > > > > To establish a full convention, I have prepared a few questions:
> > > > > >
> > > > > > 1) Do we want to use additional plugins when there is no function
> > in
> > > the
> > > > > > standard library e.g. flaky marker, forked marker? This is, in
> my
> > > > > opinion,
> > > > > > one of the big advantages of migrating to pytest.
> > > > > >
> > > > > > 2) Do we want to use custom markers?
> > > > > >
> > > > > > The discussion takes place in a separate thread:
> > > > > >
> > > > > >
> > > > > >
> > > > >
> > >
> >
> https://lists.apache.org/thread.html/4538437c96f599766005ba7829d0bee1511debb4f53599e0d300a56f%40%3Cdev.airflow.apache.org%3E
> > > > > >
> > > > > > 3) Do we want to use new assert statement?
> > > > > >
> > > > > >
> > > > > >
> > > > >
> > >
> >
> https://lists.apache.org/thread.html/08b64d3b084c865399f98f6c6f56235ce5329e843d97938e1a8045a5%40%3Cdev.airflow.apache.org%3E
> > > > > >
> > > > > > Based on the discussion with devlist, we want to delay migrations
> > to
> > > the
> > > > > > new assert statement.
> > > > > >
> > > > > > 4) Do we want to remove inheritance from unittest.TestCase?
> > > > > >
> > > > > > 5) Do we want to use class-less tests?
> > > > > >
> > > > > > 6) Do we want to use pytest function instead of current?
> > > > > >
> > > > > > For example:
> > > > > >
> > > > > > Unittest.assertRaises vs pytest.raises
> > > > > >
> > > > > > parametrize vs pytest.mark.parametrize
> > > > > >
> > > > > > unittest.skip[If], vs pytest.mark.skip[If]
> > > > > >
> > > > > > 7) Do we want to use monkeypatch fixture?
> > > > > >
> > > > > > https://docs.pytest.org/en/latest/monkeypatch.html
> > > > > >
> > > > > > 8) Do we want to use the pytest fixtures?
> > > > > >
> > > > > > Do we want to migrate all code to pytest fixtures?
> > > > > >
> > > > > > We are currently using a different style of fixtures. Do we want
> to
> > > give
> > > > > > it up?
> > > > > >
> > > > > >
> > > > >
> > >
> >
> https://docs.python.org/3/library/unittest.html#class-and-module-fixtures
> > > > > >
> > > > > >
> > > > > > It is worth asking to think about whether we do not want to
> change
> > > this
> > > > > > convention in the future e.g. after dropping support for 1.10.X.
> We
> > > can
> > > > > > also allow two conventions on a temporary basis, and then migrate
> > to
> > > one
> > > > > at
> > > > > > a later time. For example, we want to migrate to the assert
> > statement
> > > > > after
> > > > > > dropping support for 1.10
> > > > > >
> > > > > >
> > > > > > I hope I found the main differences between the current
> convention
> > > and
> > > > > the
> > > > > > new convention. However, if you missed something, please continue
> > to
> > > > > number.
> > > > > >
> > > > > >
> > > > > > Best regards,
> > > > > >
> > > > > > Kamil
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > >
> > >
> >
>
>
> --
>
> Jarek Potiuk
> Polidea <https://www.polidea.com/> | Principal Software Engineer
>
> M: +48 660 796 129 <+48660796129>
> [image: Polidea] <https://www.polidea.com/>
>
--
Tomasz Urbaszek
Polidea <https://www.polidea.com/> | Software Engineer
M: +48 505 628 493 <+48505628493>
E: tomasz.urbaszek@polidea.com <to...@polidea.com>
Unique Tech
Check out our projects! <https://www.polidea.com/our-work>
Re: Test style convention after migration to pytest
Posted by Jarek Potiuk <Ja...@polidea.com>.
> 1) Do we want to use additional plugins? *Yes*
2) Do we want to use custom markers? *Yes. They will help with optimising
> our test execution.*
3) Do we want to use new assert statement? *Yes*
> 4) Do we want to remove the inheritance from unittest.TestCase? *No
> opinion about it. I am ok with both.*
> 5) Do we want to use class-less tests? *No.*
> 6) Do we want to use pytest function instead of current? *I don't
> understand. Can you explain please?*
> 7) Do we want to use monkeypatch fixture? *No. Mock is better.*
> 8) Do we want to use the pytest fixtures? *Yes.*
>
Great that we have this discussion now. I also think we should just agree
it now and not introduce it "globally".
Once we do it, we should simply add it together new features we implement.
We have still pylint to finish as a "non-functional global change" and we
should not add new one. It's good to continually improve but one thing at a
time.
BTW Pylint goes well we are down to 243 non-pylint files from 991 since May.
J.
On Thu, Jan 2, 2020 at 12:40 AM Kaxil Naik <ka...@gmail.com> wrote:
> This is a tough one. Both arguments are reasonable.
>
> I agree at some point we should convert all to use assert. But at the same
> time, we should also focus on adding *more user-facing features *and spend
> less time on more refactor or similar changes.
>
> So based on that, this might be a low priority. We also need to still
> complete AIP-21
> <
> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-21%3A+Changes+in+import+paths
> >
> which is very critical for 2.0.
>
> 1) Do we want to use additional plugins?
>
>
> Yes.
>
> 2) Do we want to use custom markers?
>
>
> Not sure yet. Low Priority for me.
>
> 3) Do we want to use new assert statement?
>
>
> I think new PRs can contain it, shouldn't be a problem as long as it is
> documented to avoid confusion.
>
> 4) Do we want to remove the inheritance from unittest.TestCase?
>
>
> Yes, this is, however, going to change how people write tests. So someone
> has to own it as it can become painful with PRs getting merged
> continuously.
>
> 5) Do we want to use class-less tests?
>
>
> No.
>
> 6) Do we want to use pytest function instead of current?
>
>
> Yes
>
> 7) Do we want to use monkeypatch fixture?
>
>
> I also prefer unittest.mock but open to suggestions.
> https://github.com/pytest-dev/pytest/issues/4576#issuecomment-449864333
> has
> some good comparison on it
>
> 8) Do we want to use the pytest fixtures?
>
>
> Yes
>
> On Wed, Jan 1, 2020 at 8:07 PM Kamil BreguĆa <ka...@polidea.com>
> wrote:
>
> > @unittest.skip("demonstrating skipping")
> >
> > On Wed, Jan 1, 2020 at 8:37 PM Tomasz Urbaszek <tu...@apache.org>
> > wrote:
> > >
> > >
> > > > 6) Do we want to use pytest function instead of current?
> > >
> > > I do not understand this point, can you explain?
> > >
> >
> > Pytest Introduces solutions that replace solutions that are now used
> >
> > For example:
> > def test_foo(self):
> > wtih self.assertRaises(AirflowException):
> > bar()
> >
> > Can be replaced by following code:
> >
> > from pytest import raises
> >
> > wtih raises(AirflowException):
> > bar()
> >
> > OR
> >
> > from parametrize import parametrize
> >
> > @parametrize.expand([
> > (1, 1, ),
> > (2, 2, ),
> > ])
> > def test_foo(self, param_a, param_b);
> > self.assertEqual(param_a, param_b)
> >
> > can be replaced by
> >
> > @pytest.mark.parametrize("param_a,param_b", [(1, 1), (2, 2),])
> > def test_eval(param_a, param_b):
> > assert param_a == param_b
> >
> > OR
> >
> > @unittest.skip("demonstrating skipping")
> > def test_foo(self)
> >
> > can be replaced by
> >
> > @pytest.mark.skip(reason="demonstrating skipping"))
> > def test_foo(self)
> >
> > Which solution will be better for us?
> >
> > >
> > > On Wed, Jan 1, 2020 at 8:14 PM Kamil BreguĆa <
> kamil.bregula@polidea.com>
> > > wrote:
> > >
> > > > > 1) Do we want to use additional plugins?
> > > >
> > > > Yes. We should use the full-power of plugins now.
> > > >
> > > > > 2) Do we want to use custom markers?
> > > >
> > > > Reply in a separate thread.
> > > >
> > > > >3) Do we want to use new assert statement?
> > > >
> > > > Reply in a separate thread
> > > >
> > > > > 4) Do we want to remove the inheritance from unittest.TestCase?
> > > >
> > > > Yes. After dropping support for Airflow 2.0, if possible. I would
> > prefer to
> > > > focus on working on new features for Airflow 2.0.
> > > >
> > > > > 5) Do we want to use class-less tests?
> > > >
> > > > No. Classes allow easy grouping of tests. Even if a file with one
> > class now
> > > > exists, a new one may appear in the future.
> > > >
> > > > > 6) Do we want to use pytest function instead of current?
> > > >
> > > > I feel good about the current functions. However, this is not a
> serious
> > > > relationship and I can create a new friendship.
> > > >
> > > > > 7) Do we want to use monkeypatch fixture?
> > > >
> > > > No. I prefer unittest.mock
> > > >
> > > > > 8) Do we want to use the pytest fixtures?
> > > >
> > > > No. I prefer classic fixtures, if possible. Their syntax is much
> > simpler
> > > > and easier to understand.
> > > >
> > > > On Wed, Jan 1, 2020 at 8:10 PM Kamil BreguĆa <
> > kamil.bregula@polidea.com>
> > > > wrote:
> > > >
> > > > > Hello,
> > > > >
> > > > >
> > > > > We have recently migrated to pytest. Code written according to the
> > > > > standard library - unittest.TestCase is still compatible with
> pytest.
> > > > >
> > > > >
> > > > > The AIP-21 document did not discuss the migration of current code
> to
> > > > > new features, but only discussed the change of runner and benefits
> of
> > > > > pytest over nosetets.
> > > > >
> > > > > Link:
> > > > >
> > > >
> >
> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-27+Migrate+to+pytest
> > > > >
> > > > >
> > > > > As far as I appreciate the many advantages of using this tool, but
> I
> > am
> > > > > not sure **whether, how or when we want to start using some
> > features**. I
> > > > > prefer to keep the current project conventions in many areas, but I
> > still
> > > > > love pytest. I think we should establish general conventions on
> > writing
> > > > > tests and recommended solutions to known problems. I prefer a
> > pragmatic
> > > > > approach, not just the use of features, because now we can use it.
> > > > > Unfortunately, I do not see many benefits from new features.
> > > > >
> > > > >
> > > > > I would not like the code to have many ways of expressing the same
> > > > > solution. For the following reasons:
> > > > >
> > > > >
> > > > > * it can introduce a lack of understanding among new contributors
> > > > >
> > > > > * facilitate the understanding and reading of code.
> > > > >
> > > > > * creates unnecessary discussion about the preferences of one way
> > over
> > > > the
> > > > > other. Not related to changes.
> > > > >
> > > > > * forces an understanding of the complex syntax of some solutions.
> > > > >
> > > > > * encourages people to rewrite the code, which can generate
> > unnecessary
> > > > > work.
> > > > >
> > > > >
> > > > > To establish a full convention, I have prepared a few questions:
> > > > >
> > > > > 1) Do we want to use additional plugins when there is no function
> in
> > the
> > > > > standard library e.g. flaky marker, forked marker? This is, in my
> > > > opinion,
> > > > > one of the big advantages of migrating to pytest.
> > > > >
> > > > > 2) Do we want to use custom markers?
> > > > >
> > > > > The discussion takes place in a separate thread:
> > > > >
> > > > >
> > > > >
> > > >
> >
> https://lists.apache.org/thread.html/4538437c96f599766005ba7829d0bee1511debb4f53599e0d300a56f%40%3Cdev.airflow.apache.org%3E
> > > > >
> > > > > 3) Do we want to use new assert statement?
> > > > >
> > > > >
> > > > >
> > > >
> >
> https://lists.apache.org/thread.html/08b64d3b084c865399f98f6c6f56235ce5329e843d97938e1a8045a5%40%3Cdev.airflow.apache.org%3E
> > > > >
> > > > > Based on the discussion with devlist, we want to delay migrations
> to
> > the
> > > > > new assert statement.
> > > > >
> > > > > 4) Do we want to remove inheritance from unittest.TestCase?
> > > > >
> > > > > 5) Do we want to use class-less tests?
> > > > >
> > > > > 6) Do we want to use pytest function instead of current?
> > > > >
> > > > > For example:
> > > > >
> > > > > Unittest.assertRaises vs pytest.raises
> > > > >
> > > > > parametrize vs pytest.mark.parametrize
> > > > >
> > > > > unittest.skip[If], vs pytest.mark.skip[If]
> > > > >
> > > > > 7) Do we want to use monkeypatch fixture?
> > > > >
> > > > > https://docs.pytest.org/en/latest/monkeypatch.html
> > > > >
> > > > > 8) Do we want to use the pytest fixtures?
> > > > >
> > > > > Do we want to migrate all code to pytest fixtures?
> > > > >
> > > > > We are currently using a different style of fixtures. Do we want to
> > give
> > > > > it up?
> > > > >
> > > > >
> > > >
> >
> https://docs.python.org/3/library/unittest.html#class-and-module-fixtures
> > > > >
> > > > >
> > > > > It is worth asking to think about whether we do not want to change
> > this
> > > > > convention in the future e.g. after dropping support for 1.10.X. We
> > can
> > > > > also allow two conventions on a temporary basis, and then migrate
> to
> > one
> > > > at
> > > > > a later time. For example, we want to migrate to the assert
> statement
> > > > after
> > > > > dropping support for 1.10
> > > > >
> > > > >
> > > > > I hope I found the main differences between the current convention
> > and
> > > > the
> > > > > new convention. However, if you missed something, please continue
> to
> > > > number.
> > > > >
> > > > >
> > > > > Best regards,
> > > > >
> > > > > Kamil
> > > > >
> > > > >
> > > > >
> > > > >
> > > >
> >
>
--
Jarek Potiuk
Polidea <https://www.polidea.com/> | Principal Software Engineer
M: +48 660 796 129 <+48660796129>
[image: Polidea] <https://www.polidea.com/>
Re: Test style convention after migration to pytest
Posted by Kaxil Naik <ka...@gmail.com>.
This is a tough one. Both arguments are reasonable.
I agree at some point we should convert all to use assert. But at the same
time, we should also focus on adding *more user-facing features *and spend
less time on more refactor or similar changes.
So based on that, this might be a low priority. We also need to still
complete AIP-21
<https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-21%3A+Changes+in+import+paths>
which is very critical for 2.0.
1) Do we want to use additional plugins?
Yes.
2) Do we want to use custom markers?
Not sure yet. Low Priority for me.
3) Do we want to use new assert statement?
I think new PRs can contain it, shouldn't be a problem as long as it is
documented to avoid confusion.
4) Do we want to remove the inheritance from unittest.TestCase?
Yes, this is, however, going to change how people write tests. So someone
has to own it as it can become painful with PRs getting merged continuously.
5) Do we want to use class-less tests?
No.
6) Do we want to use pytest function instead of current?
Yes
7) Do we want to use monkeypatch fixture?
I also prefer unittest.mock but open to suggestions.
https://github.com/pytest-dev/pytest/issues/4576#issuecomment-449864333 has
some good comparison on it
8) Do we want to use the pytest fixtures?
Yes
On Wed, Jan 1, 2020 at 8:07 PM Kamil BreguĆa <ka...@polidea.com>
wrote:
> @unittest.skip("demonstrating skipping")
>
> On Wed, Jan 1, 2020 at 8:37 PM Tomasz Urbaszek <tu...@apache.org>
> wrote:
> >
> >
> > > 6) Do we want to use pytest function instead of current?
> >
> > I do not understand this point, can you explain?
> >
>
> Pytest Introduces solutions that replace solutions that are now used
>
> For example:
> def test_foo(self):
> wtih self.assertRaises(AirflowException):
> bar()
>
> Can be replaced by following code:
>
> from pytest import raises
>
> wtih raises(AirflowException):
> bar()
>
> OR
>
> from parametrize import parametrize
>
> @parametrize.expand([
> (1, 1, ),
> (2, 2, ),
> ])
> def test_foo(self, param_a, param_b);
> self.assertEqual(param_a, param_b)
>
> can be replaced by
>
> @pytest.mark.parametrize("param_a,param_b", [(1, 1), (2, 2),])
> def test_eval(param_a, param_b):
> assert param_a == param_b
>
> OR
>
> @unittest.skip("demonstrating skipping")
> def test_foo(self)
>
> can be replaced by
>
> @pytest.mark.skip(reason="demonstrating skipping"))
> def test_foo(self)
>
> Which solution will be better for us?
>
> >
> > On Wed, Jan 1, 2020 at 8:14 PM Kamil BreguĆa <ka...@polidea.com>
> > wrote:
> >
> > > > 1) Do we want to use additional plugins?
> > >
> > > Yes. We should use the full-power of plugins now.
> > >
> > > > 2) Do we want to use custom markers?
> > >
> > > Reply in a separate thread.
> > >
> > > >3) Do we want to use new assert statement?
> > >
> > > Reply in a separate thread
> > >
> > > > 4) Do we want to remove the inheritance from unittest.TestCase?
> > >
> > > Yes. After dropping support for Airflow 2.0, if possible. I would
> prefer to
> > > focus on working on new features for Airflow 2.0.
> > >
> > > > 5) Do we want to use class-less tests?
> > >
> > > No. Classes allow easy grouping of tests. Even if a file with one
> class now
> > > exists, a new one may appear in the future.
> > >
> > > > 6) Do we want to use pytest function instead of current?
> > >
> > > I feel good about the current functions. However, this is not a serious
> > > relationship and I can create a new friendship.
> > >
> > > > 7) Do we want to use monkeypatch fixture?
> > >
> > > No. I prefer unittest.mock
> > >
> > > > 8) Do we want to use the pytest fixtures?
> > >
> > > No. I prefer classic fixtures, if possible. Their syntax is much
> simpler
> > > and easier to understand.
> > >
> > > On Wed, Jan 1, 2020 at 8:10 PM Kamil BreguĆa <
> kamil.bregula@polidea.com>
> > > wrote:
> > >
> > > > Hello,
> > > >
> > > >
> > > > We have recently migrated to pytest. Code written according to the
> > > > standard library - unittest.TestCase is still compatible with pytest.
> > > >
> > > >
> > > > The AIP-21 document did not discuss the migration of current code to
> > > > new features, but only discussed the change of runner and benefits of
> > > > pytest over nosetets.
> > > >
> > > > Link:
> > > >
> > >
> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-27+Migrate+to+pytest
> > > >
> > > >
> > > > As far as I appreciate the many advantages of using this tool, but I
> am
> > > > not sure **whether, how or when we want to start using some
> features**. I
> > > > prefer to keep the current project conventions in many areas, but I
> still
> > > > love pytest. I think we should establish general conventions on
> writing
> > > > tests and recommended solutions to known problems. I prefer a
> pragmatic
> > > > approach, not just the use of features, because now we can use it.
> > > > Unfortunately, I do not see many benefits from new features.
> > > >
> > > >
> > > > I would not like the code to have many ways of expressing the same
> > > > solution. For the following reasons:
> > > >
> > > >
> > > > * it can introduce a lack of understanding among new contributors
> > > >
> > > > * facilitate the understanding and reading of code.
> > > >
> > > > * creates unnecessary discussion about the preferences of one way
> over
> > > the
> > > > other. Not related to changes.
> > > >
> > > > * forces an understanding of the complex syntax of some solutions.
> > > >
> > > > * encourages people to rewrite the code, which can generate
> unnecessary
> > > > work.
> > > >
> > > >
> > > > To establish a full convention, I have prepared a few questions:
> > > >
> > > > 1) Do we want to use additional plugins when there is no function in
> the
> > > > standard library e.g. flaky marker, forked marker? This is, in my
> > > opinion,
> > > > one of the big advantages of migrating to pytest.
> > > >
> > > > 2) Do we want to use custom markers?
> > > >
> > > > The discussion takes place in a separate thread:
> > > >
> > > >
> > > >
> > >
> https://lists.apache.org/thread.html/4538437c96f599766005ba7829d0bee1511debb4f53599e0d300a56f%40%3Cdev.airflow.apache.org%3E
> > > >
> > > > 3) Do we want to use new assert statement?
> > > >
> > > >
> > > >
> > >
> https://lists.apache.org/thread.html/08b64d3b084c865399f98f6c6f56235ce5329e843d97938e1a8045a5%40%3Cdev.airflow.apache.org%3E
> > > >
> > > > Based on the discussion with devlist, we want to delay migrations to
> the
> > > > new assert statement.
> > > >
> > > > 4) Do we want to remove inheritance from unittest.TestCase?
> > > >
> > > > 5) Do we want to use class-less tests?
> > > >
> > > > 6) Do we want to use pytest function instead of current?
> > > >
> > > > For example:
> > > >
> > > > Unittest.assertRaises vs pytest.raises
> > > >
> > > > parametrize vs pytest.mark.parametrize
> > > >
> > > > unittest.skip[If], vs pytest.mark.skip[If]
> > > >
> > > > 7) Do we want to use monkeypatch fixture?
> > > >
> > > > https://docs.pytest.org/en/latest/monkeypatch.html
> > > >
> > > > 8) Do we want to use the pytest fixtures?
> > > >
> > > > Do we want to migrate all code to pytest fixtures?
> > > >
> > > > We are currently using a different style of fixtures. Do we want to
> give
> > > > it up?
> > > >
> > > >
> > >
> https://docs.python.org/3/library/unittest.html#class-and-module-fixtures
> > > >
> > > >
> > > > It is worth asking to think about whether we do not want to change
> this
> > > > convention in the future e.g. after dropping support for 1.10.X. We
> can
> > > > also allow two conventions on a temporary basis, and then migrate to
> one
> > > at
> > > > a later time. For example, we want to migrate to the assert statement
> > > after
> > > > dropping support for 1.10
> > > >
> > > >
> > > > I hope I found the main differences between the current convention
> and
> > > the
> > > > new convention. However, if you missed something, please continue to
> > > number.
> > > >
> > > >
> > > > Best regards,
> > > >
> > > > Kamil
> > > >
> > > >
> > > >
> > > >
> > >
>
Re: Test style convention after migration to pytest
Posted by Kamil BreguĆa <ka...@polidea.com>.
@unittest.skip("demonstrating skipping")
On Wed, Jan 1, 2020 at 8:37 PM Tomasz Urbaszek <tu...@apache.org> wrote:
>
>
> > 6) Do we want to use pytest function instead of current?
>
> I do not understand this point, can you explain?
>
Pytest Introduces solutions that replace solutions that are now used
For example:
def test_foo(self):
wtih self.assertRaises(AirflowException):
bar()
Can be replaced by following code:
from pytest import raises
wtih raises(AirflowException):
bar()
OR
from parametrize import parametrize
@parametrize.expand([
(1, 1, ),
(2, 2, ),
])
def test_foo(self, param_a, param_b);
self.assertEqual(param_a, param_b)
can be replaced by
@pytest.mark.parametrize("param_a,param_b", [(1, 1), (2, 2),])
def test_eval(param_a, param_b):
assert param_a == param_b
OR
@unittest.skip("demonstrating skipping")
def test_foo(self)
can be replaced by
@pytest.mark.skip(reason="demonstrating skipping"))
def test_foo(self)
Which solution will be better for us?
>
> On Wed, Jan 1, 2020 at 8:14 PM Kamil BreguĆa <ka...@polidea.com>
> wrote:
>
> > > 1) Do we want to use additional plugins?
> >
> > Yes. We should use the full-power of plugins now.
> >
> > > 2) Do we want to use custom markers?
> >
> > Reply in a separate thread.
> >
> > >3) Do we want to use new assert statement?
> >
> > Reply in a separate thread
> >
> > > 4) Do we want to remove the inheritance from unittest.TestCase?
> >
> > Yes. After dropping support for Airflow 2.0, if possible. I would prefer to
> > focus on working on new features for Airflow 2.0.
> >
> > > 5) Do we want to use class-less tests?
> >
> > No. Classes allow easy grouping of tests. Even if a file with one class now
> > exists, a new one may appear in the future.
> >
> > > 6) Do we want to use pytest function instead of current?
> >
> > I feel good about the current functions. However, this is not a serious
> > relationship and I can create a new friendship.
> >
> > > 7) Do we want to use monkeypatch fixture?
> >
> > No. I prefer unittest.mock
> >
> > > 8) Do we want to use the pytest fixtures?
> >
> > No. I prefer classic fixtures, if possible. Their syntax is much simpler
> > and easier to understand.
> >
> > On Wed, Jan 1, 2020 at 8:10 PM Kamil BreguĆa <ka...@polidea.com>
> > wrote:
> >
> > > Hello,
> > >
> > >
> > > We have recently migrated to pytest. Code written according to the
> > > standard library - unittest.TestCase is still compatible with pytest.
> > >
> > >
> > > The AIP-21 document did not discuss the migration of current code to
> > > new features, but only discussed the change of runner and benefits of
> > > pytest over nosetets.
> > >
> > > Link:
> > >
> > https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-27+Migrate+to+pytest
> > >
> > >
> > > As far as I appreciate the many advantages of using this tool, but I am
> > > not sure **whether, how or when we want to start using some features**. I
> > > prefer to keep the current project conventions in many areas, but I still
> > > love pytest. I think we should establish general conventions on writing
> > > tests and recommended solutions to known problems. I prefer a pragmatic
> > > approach, not just the use of features, because now we can use it.
> > > Unfortunately, I do not see many benefits from new features.
> > >
> > >
> > > I would not like the code to have many ways of expressing the same
> > > solution. For the following reasons:
> > >
> > >
> > > * it can introduce a lack of understanding among new contributors
> > >
> > > * facilitate the understanding and reading of code.
> > >
> > > * creates unnecessary discussion about the preferences of one way over
> > the
> > > other. Not related to changes.
> > >
> > > * forces an understanding of the complex syntax of some solutions.
> > >
> > > * encourages people to rewrite the code, which can generate unnecessary
> > > work.
> > >
> > >
> > > To establish a full convention, I have prepared a few questions:
> > >
> > > 1) Do we want to use additional plugins when there is no function in the
> > > standard library e.g. flaky marker, forked marker? This is, in my
> > opinion,
> > > one of the big advantages of migrating to pytest.
> > >
> > > 2) Do we want to use custom markers?
> > >
> > > The discussion takes place in a separate thread:
> > >
> > >
> > >
> > https://lists.apache.org/thread.html/4538437c96f599766005ba7829d0bee1511debb4f53599e0d300a56f%40%3Cdev.airflow.apache.org%3E
> > >
> > > 3) Do we want to use new assert statement?
> > >
> > >
> > >
> > https://lists.apache.org/thread.html/08b64d3b084c865399f98f6c6f56235ce5329e843d97938e1a8045a5%40%3Cdev.airflow.apache.org%3E
> > >
> > > Based on the discussion with devlist, we want to delay migrations to the
> > > new assert statement.
> > >
> > > 4) Do we want to remove inheritance from unittest.TestCase?
> > >
> > > 5) Do we want to use class-less tests?
> > >
> > > 6) Do we want to use pytest function instead of current?
> > >
> > > For example:
> > >
> > > Unittest.assertRaises vs pytest.raises
> > >
> > > parametrize vs pytest.mark.parametrize
> > >
> > > unittest.skip[If], vs pytest.mark.skip[If]
> > >
> > > 7) Do we want to use monkeypatch fixture?
> > >
> > > https://docs.pytest.org/en/latest/monkeypatch.html
> > >
> > > 8) Do we want to use the pytest fixtures?
> > >
> > > Do we want to migrate all code to pytest fixtures?
> > >
> > > We are currently using a different style of fixtures. Do we want to give
> > > it up?
> > >
> > >
> > https://docs.python.org/3/library/unittest.html#class-and-module-fixtures
> > >
> > >
> > > It is worth asking to think about whether we do not want to change this
> > > convention in the future e.g. after dropping support for 1.10.X. We can
> > > also allow two conventions on a temporary basis, and then migrate to one
> > at
> > > a later time. For example, we want to migrate to the assert statement
> > after
> > > dropping support for 1.10
> > >
> > >
> > > I hope I found the main differences between the current convention and
> > the
> > > new convention. However, if you missed something, please continue to
> > number.
> > >
> > >
> > > Best regards,
> > >
> > > Kamil
> > >
> > >
> > >
> > >
> >
Re: Test style convention after migration to pytest
Posted by Tomasz Urbaszek <tu...@apache.org>.
Thanks Kamil for raising this issue! I think we can split this migration in
many simple
steps but the moment of "chaos" how we do things is unavoidable. But AIP-21
(contrib -> core) created some confusion too. But I think we are doing fine
with it.
When should we migrate? I dunno, I would like to do this asap. It will add
some more confusion
but it will end at some point. But if we postpone it to 2.0, we will finish
one "chaos" (AIP-21) and
start another one, then there could be a feeling that we are always in
chaos. But this one touches
only contributors, not users - that's a big difference.
> 1) Do we want to use additional plugins?
Yes.
> 2) Do we want to use custom markers?
Yes.
>3) Do we want to use new assert statement?
Yes.
> 4) Do we want to remove the inheritance from unittest.TestCase?
Yes. I think this can be done in two phases: I) use assert II) use pytest
fixtures
> 5) Do we want to use class-less tests?
Yes if it's a single test per file or tests "cannot be grouped" (utils,
helpers, etc).
> 6) Do we want to use pytest function instead of current?
I do not understand this point, can you explain?
> 7) Do we want to use monkeypatch fixture?
No. I too prefer unittest.mock
> 8) Do we want to use the pytest fixtures?
I am in favor of getting rid of unittest.TestCase and using pytest
fixtures. In my opinion
pytest fixtures are easier to read and thus limiting possibility of
"forgetting something in teardown".
Moreover, pytest fixtures are modular and reusable, can be defined in
conftests (many) and used with
different scopes (module, function, class).
T.
On Wed, Jan 1, 2020 at 8:14 PM Kamil BreguĆa <ka...@polidea.com>
wrote:
> > 1) Do we want to use additional plugins?
>
> Yes. We should use the full-power of plugins now.
>
> > 2) Do we want to use custom markers?
>
> Reply in a separate thread.
>
> >3) Do we want to use new assert statement?
>
> Reply in a separate thread
>
> > 4) Do we want to remove the inheritance from unittest.TestCase?
>
> Yes. After dropping support for Airflow 2.0, if possible. I would prefer to
> focus on working on new features for Airflow 2.0.
>
> > 5) Do we want to use class-less tests?
>
> No. Classes allow easy grouping of tests. Even if a file with one class now
> exists, a new one may appear in the future.
>
> > 6) Do we want to use pytest function instead of current?
>
> I feel good about the current functions. However, this is not a serious
> relationship and I can create a new friendship.
>
> > 7) Do we want to use monkeypatch fixture?
>
> No. I prefer unittest.mock
>
> > 8) Do we want to use the pytest fixtures?
>
> No. I prefer classic fixtures, if possible. Their syntax is much simpler
> and easier to understand.
>
> On Wed, Jan 1, 2020 at 8:10 PM Kamil BreguĆa <ka...@polidea.com>
> wrote:
>
> > Hello,
> >
> >
> > We have recently migrated to pytest. Code written according to the
> > standard library - unittest.TestCase is still compatible with pytest.
> >
> >
> > The AIP-21 document did not discuss the migration of current code to
> > new features, but only discussed the change of runner and benefits of
> > pytest over nosetets.
> >
> > Link:
> >
> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-27+Migrate+to+pytest
> >
> >
> > As far as I appreciate the many advantages of using this tool, but I am
> > not sure **whether, how or when we want to start using some features**. I
> > prefer to keep the current project conventions in many areas, but I still
> > love pytest. I think we should establish general conventions on writing
> > tests and recommended solutions to known problems. I prefer a pragmatic
> > approach, not just the use of features, because now we can use it.
> > Unfortunately, I do not see many benefits from new features.
> >
> >
> > I would not like the code to have many ways of expressing the same
> > solution. For the following reasons:
> >
> >
> > * it can introduce a lack of understanding among new contributors
> >
> > * facilitate the understanding and reading of code.
> >
> > * creates unnecessary discussion about the preferences of one way over
> the
> > other. Not related to changes.
> >
> > * forces an understanding of the complex syntax of some solutions.
> >
> > * encourages people to rewrite the code, which can generate unnecessary
> > work.
> >
> >
> > To establish a full convention, I have prepared a few questions:
> >
> > 1) Do we want to use additional plugins when there is no function in the
> > standard library e.g. flaky marker, forked marker? This is, in my
> opinion,
> > one of the big advantages of migrating to pytest.
> >
> > 2) Do we want to use custom markers?
> >
> > The discussion takes place in a separate thread:
> >
> >
> >
> https://lists.apache.org/thread.html/4538437c96f599766005ba7829d0bee1511debb4f53599e0d300a56f%40%3Cdev.airflow.apache.org%3E
> >
> > 3) Do we want to use new assert statement?
> >
> >
> >
> https://lists.apache.org/thread.html/08b64d3b084c865399f98f6c6f56235ce5329e843d97938e1a8045a5%40%3Cdev.airflow.apache.org%3E
> >
> > Based on the discussion with devlist, we want to delay migrations to the
> > new assert statement.
> >
> > 4) Do we want to remove inheritance from unittest.TestCase?
> >
> > 5) Do we want to use class-less tests?
> >
> > 6) Do we want to use pytest function instead of current?
> >
> > For example:
> >
> > Unittest.assertRaises vs pytest.raises
> >
> > parametrize vs pytest.mark.parametrize
> >
> > unittest.skip[If], vs pytest.mark.skip[If]
> >
> > 7) Do we want to use monkeypatch fixture?
> >
> > https://docs.pytest.org/en/latest/monkeypatch.html
> >
> > 8) Do we want to use the pytest fixtures?
> >
> > Do we want to migrate all code to pytest fixtures?
> >
> > We are currently using a different style of fixtures. Do we want to give
> > it up?
> >
> >
> https://docs.python.org/3/library/unittest.html#class-and-module-fixtures
> >
> >
> > It is worth asking to think about whether we do not want to change this
> > convention in the future e.g. after dropping support for 1.10.X. We can
> > also allow two conventions on a temporary basis, and then migrate to one
> at
> > a later time. For example, we want to migrate to the assert statement
> after
> > dropping support for 1.10
> >
> >
> > I hope I found the main differences between the current convention and
> the
> > new convention. However, if you missed something, please continue to
> number.
> >
> >
> > Best regards,
> >
> > Kamil
> >
> >
> >
> >
>
Re: Test style convention after migration to pytest
Posted by Kamil BreguĆa <ka...@polidea.com>.
> 1) Do we want to use additional plugins?
Yes. We should use the full-power of plugins now.
> 2) Do we want to use custom markers?
Reply in a separate thread.
>3) Do we want to use new assert statement?
Reply in a separate thread
> 4) Do we want to remove the inheritance from unittest.TestCase?
Yes. After dropping support for Airflow 2.0, if possible. I would prefer to
focus on working on new features for Airflow 2.0.
> 5) Do we want to use class-less tests?
No. Classes allow easy grouping of tests. Even if a file with one class now
exists, a new one may appear in the future.
> 6) Do we want to use pytest function instead of current?
I feel good about the current functions. However, this is not a serious
relationship and I can create a new friendship.
> 7) Do we want to use monkeypatch fixture?
No. I prefer unittest.mock
> 8) Do we want to use the pytest fixtures?
No. I prefer classic fixtures, if possible. Their syntax is much simpler
and easier to understand.
On Wed, Jan 1, 2020 at 8:10 PM Kamil BreguĆa <ka...@polidea.com>
wrote:
> Hello,
>
>
> We have recently migrated to pytest. Code written according to the
> standard library - unittest.TestCase is still compatible with pytest.
>
>
> The AIP-21 document did not discuss the migration of current code to
> new features, but only discussed the change of runner and benefits of
> pytest over nosetets.
>
> Link:
> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-27+Migrate+to+pytest
>
>
> As far as I appreciate the many advantages of using this tool, but I am
> not sure **whether, how or when we want to start using some features**. I
> prefer to keep the current project conventions in many areas, but I still
> love pytest. I think we should establish general conventions on writing
> tests and recommended solutions to known problems. I prefer a pragmatic
> approach, not just the use of features, because now we can use it.
> Unfortunately, I do not see many benefits from new features.
>
>
> I would not like the code to have many ways of expressing the same
> solution. For the following reasons:
>
>
> * it can introduce a lack of understanding among new contributors
>
> * facilitate the understanding and reading of code.
>
> * creates unnecessary discussion about the preferences of one way over the
> other. Not related to changes.
>
> * forces an understanding of the complex syntax of some solutions.
>
> * encourages people to rewrite the code, which can generate unnecessary
> work.
>
>
> To establish a full convention, I have prepared a few questions:
>
> 1) Do we want to use additional plugins when there is no function in the
> standard library e.g. flaky marker, forked marker? This is, in my opinion,
> one of the big advantages of migrating to pytest.
>
> 2) Do we want to use custom markers?
>
> The discussion takes place in a separate thread:
>
>
> https://lists.apache.org/thread.html/4538437c96f599766005ba7829d0bee1511debb4f53599e0d300a56f%40%3Cdev.airflow.apache.org%3E
>
> 3) Do we want to use new assert statement?
>
>
> https://lists.apache.org/thread.html/08b64d3b084c865399f98f6c6f56235ce5329e843d97938e1a8045a5%40%3Cdev.airflow.apache.org%3E
>
> Based on the discussion with devlist, we want to delay migrations to the
> new assert statement.
>
> 4) Do we want to remove inheritance from unittest.TestCase?
>
> 5) Do we want to use class-less tests?
>
> 6) Do we want to use pytest function instead of current?
>
> For example:
>
> Unittest.assertRaises vs pytest.raises
>
> parametrize vs pytest.mark.parametrize
>
> unittest.skip[If], vs pytest.mark.skip[If]
>
> 7) Do we want to use monkeypatch fixture?
>
> https://docs.pytest.org/en/latest/monkeypatch.html
>
> 8) Do we want to use the pytest fixtures?
>
> Do we want to migrate all code to pytest fixtures?
>
> We are currently using a different style of fixtures. Do we want to give
> it up?
>
> https://docs.python.org/3/library/unittest.html#class-and-module-fixtures
>
>
> It is worth asking to think about whether we do not want to change this
> convention in the future e.g. after dropping support for 1.10.X. We can
> also allow two conventions on a temporary basis, and then migrate to one at
> a later time. For example, we want to migrate to the assert statement after
> dropping support for 1.10
>
>
> I hope I found the main differences between the current convention and the
> new convention. However, if you missed something, please continue to number.
>
>
> Best regards,
>
> Kamil
>
>
>
>