I have an additional question and my reply: Do we want to allow the use of new conventions for new tests? No. I prefer keep one convention in the project.
Do we want to force the use of new conventions for new tests? No. I prefer keep one convention in the project. Do we want to allow mixed conventions as part of the same test? Example: https://github.com/apache/airflow/pull/6837 No. I prefer keep one convention in the project. Do we want to use new assert statement? Yes. After dropping support for Airflow 2.0. I would prefer to focus on working on new features for Airflow 2.0 Do we want to allow the use of new conventions for new tests? No. We should keep one project convention. We can easily migrate in the future using an automatic tools. Do we want to allow mixed conventions as part of the same test? Example: https://github.com/apache/airflow/pull/6837 Definitely no. This is too confusing. Best regards, Kamil On Wed, Jan 1, 2020 at 7:39 PM Kamil Breguła <kamil.breg...@polidea.com> wrote: > Hello, > > As I mentioned earlier, I would prefer to maintain one convention for the > project. The migration of the unittest assertions to the assert statement > can be done automatically, At the same time, it will limit discussions on > individual PRs. > > I think Tomek supports my opinion: > https://github.com/apache/airflow/pull/6837#pullrequestreview-336927276 > > Other people in the discussion also preferred to refrain from migrating to > a new style of assertion. > > For me, the discussion about **whether** we want to migrate to a new style > has ended, but we have not reached a consensus **when** we want to migrate > to a new style of the assertion. > > Best regards, > Kamil > > On Thu, Dec 19, 2019 at 3:30 PM Jarek Potiuk <jarek.pot...@polidea.com> > wrote: > >> Done. Closing the thread for now. >> >> On Wed, Dec 18, 2019 at 6:21 PM Jarek Potiuk <jarek.pot...@polidea.com> >> wrote: >> >> > Fully Agree ! I will do it then if no-one objects! >> > >> > On Wed, Dec 18, 2019 at 5:39 PM Tomasz Urbaszek < >> > tomasz.urbas...@polidea.com> wrote: >> > >> >> I agree that global implementation of asserts should happen after 2.0. >> >> But in the meantime it seems reasonable to decided on using asserts in >> the >> >> future and enforce that all new, not-conflicting tests should be >> written in >> >> pytest way, thus eliminating number of future changes. >> >> >> >> Also, after discussion with Kamil I think it would be good to describe >> >> the agreed approach in TESTING.md. >> >> >> >> T. >> >> >> >> On 2019/12/18 16:04:30, Jarek Potiuk <jarek.pot...@polidea.com> wrote: >> >> > > >> >> > > >> >> > > but I'm not >> >> > > sure which option is the best. Should we use pytest or unittest >> >> method? >> >> > > >> >> > >> >> > Yeah. This is the question that started the whole thread. So far >> only >> >> > enthusiastic "yes" for the pythonic "asserts" as far as I can see. >> >> > >> >> > Anyone has some preference for the unittest ones ? >> >> > >> >> > >> >> > > >> >> > > On Wed, Dec 18, 2019 at 2:51 PM Kaxil Naik <kaxiln...@gmail.com> >> >> wrote: >> >> > > >> >> > > > Yes I think I agree with that. We should now focus on new >> features >> >> and >> >> > > > improvements for the Users, now that we have done enough to >> improve >> >> it >> >> > > for >> >> > > > developers. >> >> > > > >> >> > > > >> >> > > > >> >> > > > On Wed, Dec 18, 2019 at 1:34 PM Kamil Breguła < >> >> kamil.breg...@polidea.com >> >> > > > >> >> > > > wrote: >> >> > > > >> >> > > > > Hello, >> >> > > > > >> >> > > > > >> >> > > > > "I'd like to hold off on something as large-scale as >> >> unittest2pytest >> >> > > > until >> >> > > > > 2.0 is out/until we no longer have to backport changes to the >> >> v1-10-* >> >> > > > > branches." ~Ash Berlin-Taylor >> >> > > > > >> >> > > > > I would also prefer to focus on working on new features for >> >> Airflow 2.0 >> >> > > > > instead of improving the developer environment. We already >> have a >> >> lot >> >> > > of >> >> > > > > changes in the environment, and the code is more solid. Users >> >> also >> >> > > > expect >> >> > > > > new features so that we don't lag in comparison to other >> >> products, e.g. >> >> > > > > Conductor, Prefetch, Dagster. >> >> > > > > >> >> > > > > Best regards, >> >> > > > > Kamil >> >> > > > > >> >> > > > > On Thu, Dec 12, 2019 at 7:58 PM Daniel Huang < >> dxhu...@gmail.com> >> >> > > wrote: >> >> > > > > >> >> > > > > > Big yes to asserts in pytest! >> >> > > > > > >> >> > > > > > On Tue, Dec 10, 2019 at 12:50 AM Michał Słowikowski < >> >> > > > > > michal.slowikow...@polidea.com> wrote: >> >> > > > > > >> >> > > > > > > I like this idea, and such assertion looks very promising. >> >> > > > > > > >> >> > > > > > > assert "maximum recursion" in str(excinfo.value) >> >> > > > > > > >> >> > > > > > > Have a nice day! >> >> > > > > > > >> >> > > > > > > >> >> > > > > > > On Mon, Dec 9, 2019 at 6:16 PM Tomasz Urbaszek < >> >> > > > > > > tomasz.urbas...@polidea.com> >> >> > > > > > > wrote: >> >> > > > > > > >> >> > > > > > > > It's definitely worth a try! I even mentioned this in >> >> AIP-27. >> >> > > But I >> >> > > > > > think >> >> > > > > > > > this script does not change SetUp / TearDown to an >> >> appropriate >> >> > > > > > fixture. >> >> > > > > > > > >> >> > > > > > > > T. >> >> > > > > > > > >> >> > > > > > > > On Mon, Dec 9, 2019 at 6:03 PM Jarek Potiuk < >> >> > > > > jarek.pot...@polidea.com> >> >> > > > > > > > wrote: >> >> > > > > > > > >> >> > > > > > > > > Well. I think (as discussed with Kamil) maybe even >> >> convert the >> >> > > > > tests >> >> > > > > > > > > automatically. >> >> > > > > > > > > >> >> > > > > > > > > How about we try: >> >> > > https://github.com/pytest-dev/unittest2pytest >> >> > > > > > > > > >> >> > > > > > > > > J. >> >> > > > > > > > > >> >> > > > > > > > > On Mon, Dec 9, 2019 at 5:48 PM Tomasz Urbaszek < >> >> > > > > > > > > tomasz.urbas...@polidea.com> wrote: >> >> > > > > > > > > >> >> > > > > > > > >> I agree. Should we encourage people to use asserts >> when >> >> adding >> >> > > > new >> >> > > > > > > > tests? >> >> > > > > > > > >> >> >> > > > > > > > >> T. >> >> > > > > > > > >> >> >> > > > > > > > >> On Mon, Dec 9, 2019 at 4:35 PM Kaxil Naik < >> >> > > kaxiln...@gmail.com> >> >> > > > > > > wrote: >> >> > > > > > > > >> >> >> > > > > > > > >>> Yup, we should. >> >> > > > > > > > >>> >> >> > > > > > > > >>> On Mon, Dec 9, 2019 at 3:20 PM Ash Berlin-Taylor < >> >> > > > a...@apache.org >> >> > > > > > >> >> > > > > > > > wrote: >> >> > > > > > > > >>> >> >> > > > > > > > >>> > Absolutely, and I don't think any of the concers >> >> about it >> >> > > in >> >> > > > > > > run-time >> >> > > > > > > > >>> code >> >> > > > > > > > >>> > apply! Plus it is the way pytest recommends, and I >> >> think we >> >> > > > get >> >> > > > > > > nicer >> >> > > > > > > > >>> > failure messages using assert-style too? >> >> > > > > > > > >>> > >> >> > > > > > > > >>> > -a >> >> > > > > > > > >>> > >> >> > > > > > > > >>> > On 9 December 2019 15:06:07 GMT, Jarek Potiuk < >> >> > > > > > > > >>> jarek.pot...@polidea.com> >> >> > > > > > > > >>> > wrote: >> >> > > > > > > > >>> > >Hello everyone. >> >> > > > > > > > >>> > > >> >> > > > > > > > >>> > >So asserts are now banned from our main code. >> >> However with >> >> > > > the >> >> > > > > > > > recent >> >> > > > > > > > >>> > >introduction of pytest we now have a chance to >> >> switch to >> >> > > > using >> >> > > > > > the >> >> > > > > > > > >>> > >standard >> >> > > > > > > > >>> > >asserts instead of deriving from TestCase class >> and >> >> using >> >> > > > > > > > >>> > >assertSomething() >> >> > > > > > > > >>> > >methods. >> >> > > > > > > > >>> > > >> >> > > > > > > > >>> > >I find it much more readable and nice and pytest >> is >> >> great >> >> > > in >> >> > > > > > > > reporting >> >> > > > > > > > >>> > >the >> >> > > > > > > > >>> > >errors in a clear and readable way. And all the >> cases >> >> > > where >> >> > > > > > > asserts >> >> > > > > > > > >>> are >> >> > > > > > > > >>> > >optimized away are not valid in this case. >> >> > > > > > > > >>> > > >> >> > > > > > > > >>> > >I think we should gradually switch to using >> asserts >> >> in our >> >> > > > > > tests. >> >> > > > > > > > >>> > > >> >> > > > > > > > >>> > >WDYT? >> >> > > > > > > > >>> > > >> >> > > > > > > > >>> > >More info: >> >> > > > > > > > >>> > > >> >> > > > > > > > >>> > >Doc about asserts in pytest: >> >> > > > > > > > >>> > >http://doc.pytest.org/en/latest/assert.html >> >> > > > > > > > >>> > > >> >> > > > > > > > >>> > >Demo of common assertion errors produced with >> pytest: >> >> > > > > > > > >>> > > >> >> > > > > > > > >>> >> >> > > > > > > > >> >> > > > > > >> >> > > > >> >> >> http://doc.pytest.org/en/latest/example/reportingdemo.html#tbreportdemo >> >> > > > > > > > >>> > > >> >> > > > > > > > >>> > >J. >> >> > > > > > > > >>> > > >> >> > > > > > > > >>> > >-- >> >> > > > > > > > >>> > > >> >> > > > > > > > >>> > >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/> | Junior 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/> >> >> > > > > > > > > >> >> > > > > > > > > >> >> > > > > > > > >> >> > > > > > > > -- >> >> > > > > > > > >> >> > > > > > > > Tomasz Urbaszek >> >> > > > > > > > Polidea <https://www.polidea.com/> | Junior 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> >> >> > > > > > > > >> >> > > > > > > >> >> > > > > > > >> >> > > > > > > -- >> >> > > > > > > >> >> > > > > > > Michał Słowikowski >> >> > > > > > > Polidea <https://www.polidea.com/> | Test Engineer >> >> > > > > > > >> >> > > > > > > E: michal.slowikow...@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/> >> >
