I think it's a good idea, but I'd augment it a bit. A better option will be to run all test types but for only one chosen combination of "Python + DB type + DB version".
I often don't even look at the PR until the tests pass and this would be much better this way. And often people have slower/small machines so they submit the PR to see if they have not broken any other tests. This is much, much easier than doing it locally - because then in one "fire&forget" you can run static, doc, unit tests, integration, and Kubernetes ones,. And it's a valid thing. Also, we have to make sure that such PR does not become "Green" before all the tests are run. This might be rather problematic as Github does not yet have a " manual" Approval step in Github Actions (it's coming in Q4: https://github.com/github/roadmap/issues/99). We have many tests and already we hit a bug a few times, where not all tests have yet started and we've merged such PR. I can imagine it will happen more and more often if all PRs will only run a subset of tests. It will be very easy to make that mistake because even if we run a subset of those tests, we have so many jobs that you cannot see them all in the GitHub UI. So we will have to have a check that fails the PR but marks it somehow as "Ready for review" for example adding a label "Ready for review" when the subset of tests succeeds. Also, this might not be needed (or less important) if we implement: https://github.com/apache/airflow/issues/10507 "Selective Tests" for which I have an open PR. They will give much bigger improvements - because, in the vast majority of cases, the tests will take very little time - giving feedback about relevant tests in a few minutes rather than half-an-hour. We can also combine those two. It seems that I managed to finish some of the stuff that I thought will take more time, so I might come back to it next week if it goes as well as I planned. J. On Fri, Oct 2, 2020 at 3:57 AM Daniel Imberman <daniel.imber...@gmail.com> wrote: > I’m not too worried about that. I think people would learn pretty quickly. > It hasn’t been an issue for the kubernetes community so I can’t imagine it > being an issue for us. End-of-day, we only have a limited amount of compute > power and this will increase the speed we merge the PR’s that have passed > basic code quality checks. > > > On Thu, Oct 1, 2020 at 2:19 PM, Tomasz Urbaszek <turbas...@apache.org> > wrote: > > I think I can agree. Especially with flaky tests, some contributors may be > confused that some of the tests don't work on CI but work locally... > > Checking the code quality is good first step. Once there's a review we can > start tests on CI. > > On the other hand, I can see people asking for starting the tests or being > even more confused why some PRs have more CI builds than others... > > Cheers, > Tomek > > On Thu, Oct 1, 2020 at 10:29 PM Daniel Imberman <daniel.imber...@gmail.com> > wrote: > >> Hello all, >> >> With the recent uptick in airflow contribution and pull requests, I have >> a proposal that I hope will ensure that we do not find ourselves in a CI >> backlog hell. I noticed that on the Kubernetes project, pull requests do >> not run integration test until a committer submits a "ready to test" >> command to the CI bot. This step can prevent draft PRs or un-reviewed PRs >> from taking github CI resources. It is worth noting that with breeze's >> docker based testing system, users have the exact same testing capabilities >> locally as they would on our CI. >> >> I propose that we allow unverified PR's to run basic and static tests, >> but not perform the full test suite or integration test without first being >> reviewed. >> >> What does everyone think? >> > -- Jarek Potiuk Polidea <https://www.polidea.com/> | Principal Software Engineer M: +48 660 796 129 <+48660796129> [image: Polidea] <https://www.polidea.com/>
