+1 to what Owen said, I don't think disabling *StressNewTest* is a good idea.
On Fri, 28 Feb 2020 at 18:35, Owen Nichols <onich...@pivotal.io> wrote: > -1 to making StressNew not required > > +1 to eliminating the current loophole — StressNew should never give a > free pass. > > Any time your PR is having trouble passing StressNew, please bring it up > on the dev list. We can review on a case-by-case basis and decide whether > to try increasing the timeout, changing the repeat count, refactoring the > PR, or as an absolute last resort requesting authorization for an override > (for example, a change to spotless rules might touch a huge number of files > but carry no risk). > > One bug we should fix is that StressNew sometimes counts more files > touched than really were, especially if you had many commits or merges or > rebases on your PR branch. Possible workarounds there include squashing > and/or creating a new PR and/or splitting into multiple PRs. I’ve spent > some time trying to reproduce why files are mis-counted, with no success, > but perhaps someone cleverer with git could provide a fix. > > Another issue is that StressNew is only in the PR pipeline, not the main > develop pipeline. This feels like an asymmetry where PRs must pass a > “higher” standard. We should consider adding some form of StressNew to the > main pipeline as well (maybe compare to the previous SHA that passed?). > > The original motivation for the 25-file limit was an attempt to limit how > long StressNew might run for. Since concourse already applies a timeout, > that check is unnecessary. However, a compromise solution might be to use > the number of files changed to dial back the number of repeats, e.g. stay > with 50 repeats if fewer than 25 files changed, otherwise compute 1250 / > <#-files-changed> and do only that many repeats (e.g. if 50 files changed, > run all tests 25x instead of 50x). > > While StressNew is intended to catch new flaky tests, it can also catch > poorly-designed tests that fail just by running twice in the same VM. This > may be a sign that the test does not clean up properly and could be > polluting other tests in unexpected ways? It might be useful to run a > “StressNew” with repeats=2 over a much broader scope, maybe even all tests, > at least once in a while? > > > > On Feb 28, 2020, at 9:51 AM, Mark Hanson <mhan...@pivotal.io> wrote: > > > > Hi All, > > > > Proposal: Force StressNewTest to fail a change with 25 or more files > rather than pass it without running it. > > > > Currently, the StressNewTest job in the pipeline will just pass a job > that has more than 25 files changed. It will be marked as green with no > work done. There are reasons, relating to run time being too long to be > tracked by concourse, so we just let it through as a pass. I think this is > a bad signal. I think that this should automatically be a failure in the > short term. As a result, I also think it should not be required. It is a > bad signal, and I think that by making it a fail, this will at least not > give us a false sense of security. I understand that this opens the flood > gates so to speak, but I don’t think as a community it is a big problem > because we can still say that you should not merge if the StressNewTest > fails because of your test. > > > > I personally find the false sense of security more troubling than > anything. Hence the reason, I would like this to be > > > > Let me know what you think.. > > > > Thanks, > > Mark > > -- Ju@N
