I understand we haven't much of a choice: +1 for disabling. I wonder if there is a way to make it more evident for developers that they need to run disabled tests locally by hand before making a PR.
It happened to me once or twice that I made a change that I knew was covered by a test case, let travis run, all green, merge! And then I discovered that the test I was relying on wasn't running on Travis and that there was a test failure when running the test locally. On Tue, Mar 12, 2019 at 9:28 AM Matthias Kuhn <[email protected]> wrote: > On 3/12/19 12:28 AM, Nyall Dawson wrote: > > On Tue, 12 Mar 2019 at 07:59, Denis Rouzaud <[email protected]> > <[email protected]> wrote: > > I am also in favor of disabling them but maybe keep them running and have > them as expected failure? > > That sounds preferable -- but I'm not (personally) sure if it's > possible. You know the CI setup better than I do, can you see a way to > do this? > > I'm not sure this works. > > Expected failure "expects" the tests to fail and will error if it > succeeds, since we are dealing with flaky tests here, that will still lead > to unstable behavior. > > If not I'd still be in favor of disabling them. > > I honestly believe the harm in leaving these enabled (short term) is > outweighing the risk of disabling them. We are very likely pushing new > contributors away from our project with the current difficulties just > getting a PR to green. > > I agree with the proposal, these tests do more harm than good currently. > > Matthias > > Nyall > > > > Cheers > Denis > > On Mon, 11 Mar 2019, 22:47 Nyall Dawson, <[email protected]> > <[email protected]> wrote: > > Hi all, > > For a long time now we've been plagued by intermittently failing tests > on Travis, which are making the whole QGIS development experience > quite painful. > > I propose that we take an absolute hard line approach from now and > disable all tests which are causing false positive failures. I've > started here: https://github.com/qgis/QGIS/pull/9483 > > This is obviously not ideal, as the failures may be revealing real > bugs (and in the case of the two disabled above I believe they are > symptoms of the same underlying bug), but I think now we've passed the > point where leaving these tests enabled causes more damage then > skipping them. > > Ideally someone would investigate these and fix either the tests or > the underlying bugs... but it hasn't happened in 6+ months, so I don't > expect that to happen shortly**. I did spend some time around a month > ago to see if the fix for these two was trivial, but could not find it > quickly. > > Is anyone opposed to a hard-line "disable if flaky" stance? > > Nyall > > ** For full disclosure: next round of QGIS grants I plan on filing for > a grant to investigate all tests disabled on Travis in depth and > either fix underlying bugs or make the tests more stable. But that's > grant dependant, and not a short term solution. > _______________________________________________ > QGIS-Developer mailing [email protected] > List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer > Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer > > -- > > Denis [email protected] > +41 76 370 21 22 > > > > _______________________________________________ > QGIS-Developer mailing [email protected] > List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer > Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer > > -- > Matthias Kuhn > [email protected] > +41 (0)76 435 67 63 <+41764356763> > [image: OPENGIS.ch Logo] <http://www.opengis.ch> > _______________________________________________ > QGIS-Developer mailing list > [email protected] > List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer > Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer -- Alessandro Pasotti w3: www.itopen.it
_______________________________________________ QGIS-Developer mailing list [email protected] List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
