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 <kamil.breg...@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 <jarek.pot...@polidea.com> > wrote: > > > > Should we resume this :)? > > > > On Mon, Jan 6, 2020 at 3:42 PM Jarek Potiuk <jarek.pot...@polidea.com> > > wrote: > > > > > Good idea! > > > > > > On Mon, Jan 6, 2020 at 3:12 PM Kamil Breguła < > kamil.breg...@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 > > >> <tomasz.urbas...@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.pot...@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 <kaxiln...@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.breg...@polidea.com> > > >> > > > wrote: > > >> > > > > > >> > > > > @unittest.skip("demonstrating skipping") > > >> > > > > > > >> > > > > On Wed, Jan 1, 2020 at 8:37 PM Tomasz Urbaszek < > > >> turbas...@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.breg...@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.breg...@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.urbas...@polidea.com <tomasz.urbasz...@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/>
