-1 on changing the check
I'm with Owen - let's leave things alone for now.
On 3/17/20, 7:43 PM, "Owen Nichols" <onich...@pivotal.io> wrote:
At least one person on the thread (@anthony) raised concerns but has not
replied since.
Also since this thread was started, the bug that miscounted files has been
fixed, which is sufficient that stress tests would have been run for about
95% of PRs that previously (erroneously) were given a free pass. Maybe
that is enough?
I’m hesitant to raise the bar for a required check if it will become an
added hardship, and especially hesitant to create a situation where the
highly-unpopular “manual override” might be the only option.
It might be better to just make slight increases from 25 files/6 hours to
35 files/10 hours, and just ask that if anyone is touching more test files
than that (due to a refactor/rename/reformat) to please make that a
separate PR separate from any actual code changes. This is better for
reviewers too...
Discussion period goes through the end of this week.
On Tue, Mar 17, 2020 at 8:55 AM Dan Smith <dsm...@pivotal.io> wrote:
> Seems like I'm the only one on this thread who even quibbled about
removing
> the limit entirely. Let's go ahead and remove the limit.
>
> -Dan
>
> On Fri, Mar 13, 2020 at 8:26 AM Robert Houghton <rhough...@pivotal.io>
> wrote:
>
> > I want the check to stay required for PR merges to be allowed. I also
> want
> > it to do real work in all cases. If I can add whitespace to 25 test
> classes
> > and get a free pass, thats a big testing hole.
> >
> > Remove the limit. Worst case, we have to A) bump timeouts in the future
> B)
> > put the limits back in C) come up with a new paradigm for verifying test
> > quality in a non-deterministic process
> >
> > On Tue, Mar 10, 2020 at 8:58 PM Dan Smith <dsm...@pivotal.io> wrote:
> >
> > > I checked, and 1% of our last 500 commits touched more than 30 tests.
> Of
> > > those 1%, over half touched more than 100 tests. So I'd guess
somewhere
> > > around .5% of prs will fall under the proposed changes.
> > >
> > > My preference would be to just raise the threshold given that we
> already
> > > raised the timeout. That'll catch 99% of prs without making test
> > > refactoring and cleanups that touch many tests hard.
> > >
> > > But, I'm ok with removing the limit if people really feel that a
manual
> > > override process is better for that 1%.
> > >
> > > -Dan
> > > On Tue, Mar 10, 2020, 4:33 PM Owen Nichols <onich...@pivotal.io>
> wrote:
> > >
> > > > To recap, the original question was: should we change StressNewTest
> to
> > > > pass ONLY if it is able to successfully run all new/changed tests 50
> > > times
> > > > (eliminating the existing loophole exempting PRs that touch 25 or
> more
> > > > files)?
> > > >
> > > > So far a handful of people have expressed support, but many have
> > > > questions/concerns:
> > > > - One concern was mis-counting of test files touched. As of today
> this
> > > is
> > > > now fixed.
> > > > - A big concern was that it might become more difficult to pass all
> > > > required PR checks.
> > > > - Another concern was that the current timeout of 6 hours might be
> too
> > > > high / too low.
> > > > - An alternative was suggested to keep the loophole but maybe
> increase
> > > the
> > > > threshold required to get the free pass.
> > > > - If we’re going to raise the bar on any required PR check, maybe we
> > > > should consider making it non-required.
> > > >
> > > > Let’s extend the comment period until the end of next week (Friday
> Mar
> > > 20)
> > > > in hopes of converging on a solution everybody is happy with (even
if
> > it
> > > > isn’t what was originally proposed).
> > > >
> > > >
> > > > > On Mar 6, 2020, at 4:51 PM, Donal Evans <doev...@pivotal.io>
> wrote:
> > > > >
> > > > > With fairly apt timing, we've recently had a PR merged (
> > > > > https://github.com/apache/geode/pull/4745) that introduced a test
> > that
> > > > has
> > > > > resulted in fairly consistently flaky failures in the pipeline
> (JIRA
> > > > > ticket:
> > > https://issues.apache.org/jira/projects/GEODE/issues/GEODE-7843
> > > > ).
> > > > > The PR was quite large and touched/created a lot of tests, so
> > > > StressNewTest
> > > > > never ran on it:
> > > > > https://concourse.apachegeode-ci.info/builds/136784#L5e3ac3fc:2.
> > Given
> > > > how
> > > > > frequently the test is failing in the pipeline (11 failures on
> > various
> > > > > IntegrationTest jobs over the past 6 commits), it seems very
likely
> > > that
> > > > > had StressNewTest been allowed to run, this issue would have been
> > > caught
> > > > at
> > > > > the PR stage and could have been remedied then, instead of
> resulting
> > > in a
> > > > > nearly constantly red pipeline.
> > > > >
> > > > > I've already given my +1 to this proposal, but this situation has
> > > > prompted
> > > > > me to make it a +2.
> > > > >
> > > > > On Fri, Mar 6, 2020 at 1:46 PM Donal Evans <doev...@pivotal.io>
> > wrote:
> > > > >
> > > > >> With fairly apt timing, we've recently had a PR merged (
> > > > >> https://github.com/apache/geode/pull/4745) that introduced a test
> > > that
> > > > >> has resulted in fairly consistently flaky failures in the
pipeline
> > > (JIRA
> > > > >> ticket:
> > > https://issues.apache.org/jira/projects/GEODE/issues/GEODE-7843
> > > > ).
> > > > >> The PR was quite large and touched/created a lot of tests, so
> > > > StressNewTest
> > > > >> never ran on it:
> > > > >> https://concourse.apachegeode-ci.info/builds/136784#L5e3ac3fc:2.
> > > Given
> > > > >> how frequently the test is failing in the pipeline (11 failures
on
> > > > various
> > > > >> IntegrationTest jobs over the past 6 commits), it seems very
> likely
> > > that
> > > > >> had StressNewTest been allowed to run, this issue would have been
> > > > caught at
> > > > >> the PR stage and could have been remedied then, instead of
> resulting
> > > in
> > > > a
> > > > >> nearly constantly red pipeline.
> > > > >>
> > > > >> I've already given my +1 to this proposal, but this situation has
> > > > prompted
> > > > >> me to make it a *+1*.
> > > > >>
> > > > >> On Tue, Mar 3, 2020 at 2:08 PM Anilkumar Gingade <
> > aging...@pivotal.io
> > > >
> > > > >> wrote:
> > > > >>
> > > > >>> The stress test is to identify the flaky-ness within the test;
> and
> > > > >>> assuming
> > > > >>> any changes to the test may have introduced the flaky-ness.
> > > > >>> It's about paying the cost upfront or later when the test is
> > > > determined to
> > > > >>> be flaky.
> > > > >>> If 25+ tests have been changed in a PR, the cost of running
> stress
> > > test
> > > > >>> for
> > > > >>> those; and gating the PR for so long.
> > > > >>> Knowing how much pain it causes to fix a flaky test after a
> > > > certain/long
> > > > >>> duration of time; I am +1 for doing this change.
> > > > >>>
> > > > >>> On Tue, Mar 3, 2020 at 10:06 AM Dan Smith <dsm...@pivotal.io>
> > wrote:
> > > > >>>
> > > > >>>> What's the current timeout for StressNewTest? Maybe if we just
> up
> > > the
> > > > >>>> threshold to 100 tests or so and up the timeout to match we can
> > > catch
> > > > >>>> pretty much all PRs.
> > > > >>>>
> > > > >>>> I'm not sure why the job is flagging more tests than it should.
> It
> > > > looks
> > > > >>>> like at some point @rhoughon changed it to read the merge base
> > from
> > > > some
> > > > >>>> file created by concourse as an optimization [1] - I suspect
> maybe
> > > > that
> > > > >>>> file is inaccurate?
> > > > >>>>
> > > > >>>> I originally wrote this job. It's definitely not a panacea, it
> > will
> > > > only
> > > > >>>> catch a new flaky test if
> > > > >>>> - the test is really flaky (likely to fail more than 1/50
times)
> > > > >>>> - the change actually happened in the test file itself, and not
> > the
> > > > >>>> product or some other test file.
> > > > >>>>
> > > > >>>> [1]
> > > > >>>>
> > > > >>>>
> > > > >>>
> > > >
> > >
> >
>
https://github.com/apache/geode/commit/4c06ba4625e69d44a5165aa9f2fccddfc064de87
> > > > >>>>
> > > > >>>> -Dan
> > > > >>>>
> > > > >>>> On Sun, Mar 1, 2020 at 9:00 PM Owen Nichols <
> onich...@pivotal.io>
> > > > >>> wrote:
> > > > >>>>
> > > > >>>>> We don’t tend to look too closely at successful PR checks to
> see
> > > > >>> whether
> > > > >>>>> they actually checked anything at all.
> > > > >>>>>
> > > > >>>>> One example I found is
> > > > >>>>>
> > > > >>>>
> > > > >>>
> > > >
> > >
> >
>
https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-pr/jobs/StressNewTestOpenJDK11/builds/5957
> > > > >>>>> <
> > > > >>>>>
> > > > >>>>
> > > > >>>
> > > >
> > >
> >
>
https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-pr/jobs/StressNewTestOpenJDK11/builds/5957
> > > > >>>>>> :
> > > > >>>>> 32 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing.
> > > > >>>>>
> > > > >>>>> Here are 92 more examples (url’s omitted for brevity — use the
> > > > example
> > > > >>>>> above as a template and just replace the last 4 digits):
> > > > >>>>> 26 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6243)
> > > > >>>>> 26 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6249)
> > > > >>>>> 26 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6402)
> > > > >>>>> 27 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6262)
> > > > >>>>> 27 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6430)
> > > > >>>>> 27 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6439)
> > > > >>>>> 27 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6449)
> > > > >>>>> 27 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6454)
> > > > >>>>> 27 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6458)
> > > > >>>>> 27 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6459)
> > > > >>>>> 28 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6224)
> > > > >>>>> 28 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6441)
> > > > >>>>> 28 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6448)
> > > > >>>>> 28 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6452)
> > > > >>>>> 29 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6102)
> > > > >>>>> 29 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6177)
> > > > >>>>> 30 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 5939)
> > > > >>>>> 30 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 5940)
> > > > >>>>> 30 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 5949)
> > > > >>>>> 30 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6473)
> > > > >>>>> 31 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 5953)
> > > > >>>>> 31 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6187)
> > > > >>>>> 31 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6470)
> > > > >>>>> 31 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6471)
> > > > >>>>> 31 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6474)
> > > > >>>>> 31 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6475)
> > > > >>>>> 32 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 5958)
> > > > >>>>> 32 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6173)
> > > > >>>>> 32 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6236)
> > > > >>>>> 32 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6237)
> > > > >>>>> 32 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6242)
> > > > >>>>> 33 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6246)
> > > > >>>>> 33 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6248)
> > > > >>>>> 33 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6250)
> > > > >>>>> 33 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6251)
> > > > >>>>> 33 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6254)
> > > > >>>>> 33 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6255)
> > > > >>>>> 34 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6139)
> > > > >>>>> 34 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6141)
> > > > >>>>> 34 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6264)
> > > > >>>>> 34 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6266)
> > > > >>>>> 34 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6271)
> > > > >>>>> 35 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6142)
> > > > >>>>> 35 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6143)
> > > > >>>>> 35 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6277)
> > > > >>>>> 35 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6309)
> > > > >>>>> 35 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6413)
> > > > >>>>> 35 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6414)
> > > > >>>>> 35 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6419)
> > > > >>>>> 35 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6420)
> > > > >>>>> 36 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6144)
> > > > >>>>> 36 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6146)
> > > > >>>>> 36 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6147)
> > > > >>>>> 36 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6310)
> > > > >>>>> 36 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6326)
> > > > >>>>> 36 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6329)
> > > > >>>>> 36 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6330)
> > > > >>>>> 40 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6159)
> > > > >>>>> 40 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6160)
> > > > >>>>> 40 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6188)
> > > > >>>>> 41 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6440)
> > > > >>>>> 41 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6480)
> > > > >>>>> 41 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6483)
> > > > >>>>> 43 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6190)
> > > > >>>>> 44 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 5768)
> > > > >>>>> 44 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 5772)
> > > > >>>>> 46 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6191)
> > > > >>>>> 46 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6218)
> > > > >>>>> 46 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6219)
> > > > >>>>> 46 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6225)
> > > > >>>>> 46 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6244)
> > > > >>>>> 46 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6245)
> > > > >>>>> 48 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6129)
> > > > >>>>> 49 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 5804)
> > > > >>>>> 51 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 5877)
> > > > >>>>> 53 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 5742)
> > > > >>>>> 54 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 5757)
> > > > >>>>> 66 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6232)
> > > > >>>>> 66 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6233)
> > > > >>>>> 66 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6234)
> > > > >>>>> 66 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6259)
> > > > >>>>> 66 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6260)
> > > > >>>>> 66 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6278)
> > > > >>>>> 79 is too many changed tests to stress test. Allowing this job
> to
> > > > pass
> > > > >>>>> without stress testing. (build 6481)
> > > > >>>>> 103 is too many changed tests to stress test. Allowing this
job
> > to
> > > > >>> pass
> > > > >>>>> without stress testing. (build 6493)
> > > > >>>>> 106 is too many changed tests to stress test. Allowing this
job
> > to
> > > > >>> pass
> > > > >>>>> without stress testing. (build 6206)
> > > > >>>>> 106 is too many changed tests to stress test. Allowing this
job
> > to
> > > > >>> pass
> > > > >>>>> without stress testing. (build 6207)
> > > > >>>>> 115 is too many changed tests to stress test. Allowing this
job
> > to
> > > > >>> pass
> > > > >>>>> without stress testing. (build 5769)
> > > > >>>>> 118 is too many changed tests to stress test. Allowing this
job
> > to
> > > > >>> pass
> > > > >>>>> without stress testing. (build 6226)
> > > > >>>>> 721 is too many changed tests to stress test. Allowing this
job
> > to
> > > > >>> pass
> > > > >>>>> without stress testing. (build 6197)
> > > > >>>>> 722 is too many changed tests to stress test. Allowing this
job
> > to
> > > > >>> pass
> > > > >>>>> without stress testing. (build 6217)
> > > > >>>>> 722 is too many changed tests to stress test. Allowing this
job
> > to
> > > > >>> pass
> > > > >>>>> without stress testing. (build 6221)
> > > > >>>>>
> > > > >>>>> Please note, sometimes the number of files reported seems to
be
> > way
> > > > >>>> higher
> > > > >>>>> than it should be. For example
> > > > >>>> https://github.com/apache/geode/pull/4691
> > > > >>>>> <https://github.com/apache/geode/pull/4691> shows exactly 1
> test
> > > > file
> > > > >>>>> touched, but
> > > > >>>>>
> > > > >>>>
> > > > >>>
> > > >
> > >
> >
>
https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-pr/jobs/StressNewTestOpenJDK11/builds/6278
> > > > >>>>> <
> > > > >>>>>
> > > > >>>>
> > > > >>>
> > > >
> > >
> >
>
https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-pr/jobs/StressNewTestOpenJDK11/builds/6278
> > > > >>>>>
> > > > >>>>> thinks it touched 66 test files.
> > > > >>>>>
> > > > >>>>> There’s currently no good data to predict how long StressNew
> will
> > > > >>> take,
> > > > >>>> it
> > > > >>>>> just depends on the mix of tests touched. I am aware of at
> > least 4
> > > > >>> cases
> > > > >>>>> (with less than 25 files) where the timeout was hit. In two
of
> > > those
> > > > >>>> cases
> > > > >>>>> we re-ran with a longer timeout, and in two cases the PR
author
> > > split
> > > > >>> up
> > > > >>>>> the PR into half a dozen smaller PRs to get around it.
> > > > >>>>>
> > > > >>>>>
> > > > >>>>>> On Mar 1, 2020, at 7:52 PM, Anthony Baker <aba...@pivotal.io>
> > > > >>> wrote:
> > > > >>>>>>
> > > > >>>>>> What percentage of PR’s are currently subject to the 25 test
> > file
> > > > >>> rule?
> > > > >>>>> How many would be subject to the concourse timeout?
> > > > >>>>>>
> > > > >>>>>> I’d like to understand the scope of the impact for this
> change.
> > > > >>>>>>
> > > > >>>>>> Anthony
> > > > >>>>>>
> > > > >>>>>>
> > > > >>>>>>> On Mar 1, 2020, at 8:58 AM, Owen Nichols <
> onich...@pivotal.io>
> > > > >>> wrote:
> > > > >>>>>>>
> > > > >>>>>>> Impossible, no. Inconvenient, perhaps, but a small price to
> pay
> > > for
> > > > >>>>> being
> > > > >>>>>>> able to trust that green means green.
> > > > >>>>>>>
> > > > >>>>>>> With or without this proposed change, 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 changed test
files,
> > > > >>>> splitting
> > > > >>>>>>> into multiple PRs, authorizing an override, etc).
> > > > >>>>>>>
> > > > >>>>>>> On Sun, Mar 1, 2020 at 8:56 AM Dan Smith <dsm...@pivotal.io>
> > > > >>> wrote:
> > > > >>>>>>>
> > > > >>>>>>>> Won't this make it impossible to merge refactoring changes
> > that
> > > > >>> touch
> > > > >>>>> a lot
> > > > >>>>>>>> of tests?
> > > > >>>>>>>>
> > > > >>>>>>>> -Dan
> > > > >>>>>>>>
> > > > >>>>>>>> On Sat, Feb 29, 2020, 12:37 PM Robert Houghton <
> > > > >>> rhough...@pivotal.io
> > > > >>>>>
> > > > >>>>>>>> wrote:
> > > > >>>>>>>>
> > > > >>>>>>>>> Yes, as it should
> > > > >>>>>>>>>
> > > > >>>>>>>>> On Sat, Feb 29, 2020, 12:25 Dan Smith <dsm...@pivotal.io>
> > > wrote:
> > > > >>>>>>>>>
> > > > >>>>>>>>>> Doesn't the build fail when concourse times out?
> > > > >>>>>>>>>
> > > > >>>>>>>>>
> > > > >>>>>>>>
> > > > >>>>>>
> > > > >>>>>
> > > > >>>>>
> > > > >>>>
> > > > >>>
> > > > >>
> > > >
> > > >
> > >
> >
>
