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