I like the idea of a separate concurrency test suite. We have other tests of concurrency that aren't using this runner that could also go there - maybe we could establish some good conventions and figure out how to give more CPU time to these tests. I'd actually like to see *more* tests of concurrency at a lower level, so we can catch race conditions sooner.
> Should I disable these tests? I don't think we should be marking any tests as ignored. Let's figure out what is going on and make the appropriate fixes. Can you clarify why these tests are blocking your Cache.close PR? Also, which tests are getting an OOME with the mockito PR? For a little clarity on the ConcurrentTestRunner, it just runs a test with multiple threads some number of times. For example FilterProfileConcurrencyTest.serializationOfFilterProfileWithConcurrentUpdateShouldSucceed is testing a bug some of our customers hit where serializing an object that was being concurrently modified failed. I think there is definitely value in having those sorts of tests given the type of code we have - a lot of classes that are mutable and need to be thread safe. -Dan On Thu, Apr 16, 2020 at 2:47 PM Jacob Barrett <jbarr...@pivotal.io> wrote: > > > > On Apr 16, 2020, at 2:16 PM, Kirk Lund <kl...@apache.org> wrote: > > > Anyone else up for moving them to new src sets? Unfortunately, this alone > > won't make the tests work with the newer versions of Mockito -- and guess > > what, they only OOME in the cloud (not locally). > > I could take a stab at it since I split up all the test types originally. > I should be able to brush off those cobwebs. > > > Should I disable these tests? > > In an effort to get the Mockito stuff through, yes disable them. > > > Should I remove the use of ConcurrentTestRunner from these tests > > (converting them to simple junit tests)? > > > > I spent some time studying ConcurrentTestRunner and I'm sorry, but I > don't > > really see the value in this runner, which is why I want to remove the > use > > of it and convert these tests to plain junit tests. > > They have less value as unit tests since there are already unit tests that > cover them. It does raise a good questions about the value and > implementation of micro concurrent testing like this. I think isolating > them away is a good step before starting this conversation. > > >
