As @Matthias Pohl <matth...@ververica.com> mentioned, I agree that no1 is to end up with consistency regarding the assertions in our tests, but I also like how those assertions shape up with the AssertJ approach.
On Thu, Nov 25, 2021 at 9:38 AM Francesco Guardiani <france...@ververica.com> wrote: > This is the result of experimenting around creating custom assertions for > Table API types > https://github.com/slinkydeveloper/flink/commit/ > d1ce37a62c2200b2c3008a9cc2cac91234222fd5[1]. I will PR it once the two PRs > in the > previous mail get merged > > On Monday, 22 November 2021 17:59:29 CET Francesco Guardiani wrote: > > Hi all, > > > > Given I see generally consensus around having a convention and using > > assertj, I propose to merge these 2 PRs: > > > > * Add the explanation of this convention in our code quality guide: > > https://github.com/apache/flink-web/pull/482 > > * Add assertj to dependency management in the parent pom and link in the > PR > > template the code quality guide: > https://github.com/apache/flink/pull/17871 > > > > WDYT? > > > > Once we merge those, I'll work in the next days to add some custom > > assertions in table-common for RowData and Row (commonly asserted > > everywhere in the table codebase). > > > > @Matthias Pohl <matth...@ververica.com> about the confluence page, it > seems > > a bit outdated, judging from the last modified date. I propose to > continue > > to use this guide > > https://flink.apache.org/contributing/code-style-and-quality-common.html > as > > it seems more complete. > > > > > > On Mon, Nov 22, 2021 at 8:58 AM Matthias Pohl <matth...@ververica.com> > > > > wrote: > > > Agree. Clarifying once more what our preferred option is here, is a > good > > > idea. So, +1 for unification. I don't have a strong opinion on what > > > framework to use. But we may want to add this at the end of the > discussion > > > to our documentation (e.g. [1] or maybe the PR description?) to make > users > > > aware of it and be able to provide a reference in case it comes up > again > > > (besides this ML thread). Or do we already have something like that > > > somewhere in the docs where I missed it? > > > > > > Matthias > > > > > > [1] > > > > https://cwiki.apache.org/confluence/display/FLINK/Best+Practices+and+Lesso > > > ns+Learned> > > > On Wed, Nov 17, 2021 at 11:13 AM Marios Trivyzas <mat...@gmail.com> > wrote: > > >> I'm also +1 both for unification and specifically for assertJ. > > >> I think it covers a wide variety of assertions and as Francesco > mentioned > > >> it's easily extensible, so that > > >> we can create custom assertions where needed, and avoid repeating test > > >> code. > > >> > > >> On Tue, Nov 16, 2021 at 9:57 AM David Morávek <d...@apache.org> > wrote: > > >> > I don't have any strong opinions on the asserting framework that we > > >> > use, > > >> > but big +1 for the unification. > > >> > > > >> > Best, > > >> > D. > > >> > > > >> > On Tue, Nov 16, 2021 at 9:37 AM Till Rohrmann <trohrm...@apache.org > > > > >> > > > >> > wrote: > > >> > > Using JUnit5 with assertJ is fine with me if the community agrees. > > >> > > >> Having > > >> > > >> > > guides for best practices would definitely help with the > transition. > > >> > > > > >> > > Cheers, > > >> > > Till > > >> > > > > >> > > On Mon, Nov 15, 2021 at 5:34 PM Francesco Guardiani < > > >> > > france...@ververica.com> > > >> > > > > >> > > wrote: > > >> > > > > It is a bit unfortunate that we have tests that follow > different > > >> > > > > > >> > > > patterns. > > >> > > > This, however, is mainly due to organic growth. I think the > > >> > > >> community > > >> > > >> > > > started with Junit4, then we chose to use Hamcrest because of > its > > >> > > > >> > better > > >> > > > >> > > > expressiveness. > > >> > > > > > >> > > > That is fine, I'm sorry if my mail felt like a rant :) > > >> > > > > > >> > > > > Personally, I don't have a strong preference for which testing > > >> > > >> tools > > >> > > >> > to > > >> > > > >> > > > use. The important bit is that we agree as a community, then > > >> > > >> document > > >> > > >> > the > > >> > > > >> > > > choice and finally stick to it. So before starting to use > assertj, > > >> > > >> we > > >> > > >> > > > should probably align with the folks working on the Junit5 > effort > > >> > > > >> > first. > > >> > > > >> > > > As Arvid pointed out, using assertj might help the people > working -- Marios
