Re: [PROPOSAL] eliminate file count loophole in PR StressNewTest
Doesn't the build fail when concourse times out? -Dan On Fri, Feb 28, 2020, 1:37 PM Donal Evans wrote: > +1 > > If the original reason for the 25 file limit was to prevent StressNewTest > taking too long, then the concourse timeout renders the restriction > redundant. Also, I'd think that if anything, the more files a PR changed, > the more interested we would be in running StressNewTest. Allowing it to > just be "green" when we get over 25 changed files seems counterintuitive > with that in mind. > > On Fri, Feb 28, 2020 at 12:04 PM Owen Nichols wrote: > > > Currently, StressNewTest will turn green WITHOUT RUNNING ANY TESTS if it > > thinks (often wrongly) that more than 25 test files were touched. > > > > This is both dishonest (it can give a false green) AND totally > unnecessary > > (concourse already applies a timeout to StressNewTest, currently set at 6 > > hours, so there is no reason not to at least try to run all changed > tests, > > they may easily complete in under 6 hours even if many files). > > > > *** I PROPOSE that we should remove this 25-file-free-pass loophole. *** > > > > As always, if anyone is having trouble getting their PR to pass > StressNew, > > please bring it up on the dev list and we can discuss the appropriate > > solution on a case-by-case basis (e.g. increasing timeout, fixing the > logic > > that identifies changes test files, splitting into multiple PRs, etc). > > > > >
Re: [PROPOSAL] eliminate file count loophole in PR StressNewTest
Yes, that is the idea. No more false greens! > On Feb 29, 2020, at 12:25 PM, Dan Smith wrote: > > Doesn't the build fail when concourse times out? > > -Dan > > On Fri, Feb 28, 2020, 1:37 PM Donal Evans wrote: > >> +1 >> >> If the original reason for the 25 file limit was to prevent StressNewTest >> taking too long, then the concourse timeout renders the restriction >> redundant. Also, I'd think that if anything, the more files a PR changed, >> the more interested we would be in running StressNewTest. Allowing it to >> just be "green" when we get over 25 changed files seems counterintuitive >> with that in mind. >> >> On Fri, Feb 28, 2020 at 12:04 PM Owen Nichols wrote: >> >>> Currently, StressNewTest will turn green WITHOUT RUNNING ANY TESTS if it >>> thinks (often wrongly) that more than 25 test files were touched. >>> >>> This is both dishonest (it can give a false green) AND totally >> unnecessary >>> (concourse already applies a timeout to StressNewTest, currently set at 6 >>> hours, so there is no reason not to at least try to run all changed >> tests, >>> they may easily complete in under 6 hours even if many files). >>> >>> *** I PROPOSE that we should remove this 25-file-free-pass loophole. *** >>> >>> As always, if anyone is having trouble getting their PR to pass >> StressNew, >>> please bring it up on the dev list and we can discuss the appropriate >>> solution on a case-by-case basis (e.g. increasing timeout, fixing the >> logic >>> that identifies changes test files, splitting into multiple PRs, etc). >>> >>> >>
Re: [PROPOSAL] eliminate file count loophole in PR StressNewTest
Yes, as it should On Sat, Feb 29, 2020, 12:25 Dan Smith wrote: > Doesn't the build fail when concourse times out? > > -Dan > > On Fri, Feb 28, 2020, 1:37 PM Donal Evans wrote: > > > +1 > > > > If the original reason for the 25 file limit was to prevent StressNewTest > > taking too long, then the concourse timeout renders the restriction > > redundant. Also, I'd think that if anything, the more files a PR changed, > > the more interested we would be in running StressNewTest. Allowing it to > > just be "green" when we get over 25 changed files seems counterintuitive > > with that in mind. > > > > On Fri, Feb 28, 2020 at 12:04 PM Owen Nichols > wrote: > > > > > Currently, StressNewTest will turn green WITHOUT RUNNING ANY TESTS if > it > > > thinks (often wrongly) that more than 25 test files were touched. > > > > > > This is both dishonest (it can give a false green) AND totally > > unnecessary > > > (concourse already applies a timeout to StressNewTest, currently set > at 6 > > > hours, so there is no reason not to at least try to run all changed > > tests, > > > they may easily complete in under 6 hours even if many files). > > > > > > *** I PROPOSE that we should remove this 25-file-free-pass loophole. > *** > > > > > > As always, if anyone is having trouble getting their PR to pass > > StressNew, > > > please bring it up on the dev list and we can discuss the appropriate > > > solution on a case-by-case basis (e.g. increasing timeout, fixing the > > logic > > > that identifies changes test files, splitting into multiple PRs, etc). > > > > > > > > >
