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