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/>
