It is a good principle to run all tests successfully with any change. This means a lot for project's stability and development. I am big +1 for this proposal.
Best liujiangang Till Rohrmann <trohrm...@apache.org> 于2021年6月22日周二 下午6:36写道： > One way to address the problem of regularly failing tests that block > merging of PRs is to disable the respective tests for the time being. Of > course, the failing test then needs to be fixed. But at least that way we > would not block everyone from making progress. > > Cheers, > Till > > On Tue, Jun 22, 2021 at 12:00 PM Arvid Heise <ar...@apache.org> wrote: > > > I think this is overall a good idea. So +1 from my side. > > However, I'd like to put a higher priority on infrastructure then, in > > particular docker image/artifact caches. > > > > On Tue, Jun 22, 2021 at 11:50 AM Till Rohrmann <trohrm...@apache.org> > > wrote: > > > > > Thanks for bringing this topic to our attention Xintong. I think your > > > proposal makes a lot of sense and we should follow it. It will give us > > > confidence that our changes are working and it might be a good > incentive > > to > > > quickly fix build instabilities. Hence, +1. > > > > > > Cheers, > > > Till > > > > > > On Tue, Jun 22, 2021 at 11:12 AM Xintong Song <tonysong...@gmail.com> > > > wrote: > > > > > > > Hi everyone, > > > > > > > > In the past a couple of weeks, I've observed several times that PRs > are > > > > merged without a green light from the CI tests, where failure cases > are > > > > considered *unrelated*. This may not always cause problems, but would > > > > increase the chance of breaking our code base. In fact, it has > occurred > > > to > > > > me twice in the past few weeks that I had to revert a commit which > > breaks > > > > the master branch due to this. > > > > > > > > I think it would be nicer to enforce a stricter rule, that no PRs > > should > > > be > > > > merged without passing CI. > > > > > > > > The problems of merging PRs with "unrelated" test failures are: > > > > - It's not always straightforward to tell whether a test failures are > > > > related or not. > > > > - It prevents subsequent test cases from being executed, which may > fail > > > > relating to the PR changes. > > > > > > > > To make things easier for the committers, the following exceptions > > might > > > be > > > > considered acceptable. > > > > - The PR has passed CI in the contributor's personal workspace. > Please > > > post > > > > the link in such cases. > > > > - The CI tests have been triggered multiple times, on the same > commit, > > > and > > > > each stage has at least passed for once. Please also comment in such > > > cases. > > > > > > > > If we all agree on this, I'd update the community guidelines for > > merging > > > > PRs wrt. this proposal. [1] > > > > > > > > Please let me know what do you think. > > > > > > > > Thank you~ > > > > > > > > Xintong Song > > > > > > > > > > > > [1] > > > > > > https://cwiki.apache.org/confluence/display/FLINK/Merging+Pull+Requests > > > > > > > > > >
