On Sat, Mar 18, 2017 at 9:21 PM, Qingcun Zhou <zhouqing...@gmail.com> wrote:
> I wanted to contribute some unit test cases. However the unit test approach > in Cassandra seems weird to me after looking into some examples. Not sure > if anyone else has the same feeling. > > Usually, at least for all Java projects I have seen, people use mock > (mockito, powermock) for dependencies. And then in a particular test case > you verify the behavior using junit.assert* or mockito.verify. However we > don't use mockito in Cassandra. Is there any reason for this? Without > these, how easy do people think about adding unit test cases? > > > Besides that, we have lots of singletons and there are already a handful of > tickets to eliminate them. Maybe I missed something but I'm not seeing much > progress. Is anyone actively working on this? > > Maybe a related problem. Some unit test cases have method annotated with > @BeforeClass to do initialization work. However, it not only initializes > direct dependencies, but also indirect ones, including loading > cassandra.yaml and initializing indirect dependencies. This seems to me > more like functional/integration test but not unit test style. > > > On Fri, Mar 17, 2017 at 2:56 PM, Jeremy Hanna <jeremy.hanna1...@gmail.com> > wrote: > > > https://issues.apache.org/jira/browse/CASSANDRA-7837 may be some > > interesting context regarding what's been worked on to get rid of > > singletons and static initialization. > > > > > On Mar 17, 2017, at 4:47 PM, Jonathan Haddad <j...@jonhaddad.com> > wrote: > > > > > > I'd like to think that if someone refactors existing code, making it > more > > > testable (with tests, of course) it should be acceptable on it's own > > > merit. In fact, in my opinion it sometimes makes more sense to do > these > > > types of refactorings for the sole purpose of improving stability and > > > testability as opposed to mixing them with features. > > > > > > You referenced the issue I fixed in one of the early emails. The fix > > > itself was a couple lines of code. Refactoring the codebase to make it > > > testable would have been a huge effort. I wish I had time to do it. I > > > created CASSANDRA-13007 as a follow up with the intent of working on > > > compaction from a purely architectural standpoint. I think this type > of > > > thing should be done throughout the codebase. > > > > > > Removing the singletons is a good first step, my vote is we just rip > off > > > the bandaid, do it, and move forward. > > > > > > On Fri, Mar 17, 2017 at 2:20 PM Edward Capriolo <edlinuxg...@gmail.com > > > > > wrote: > > > > > >>> On Fri, Mar 17, 2017 at 2:31 PM, Jason Brown <jasedbr...@gmail.com> > > wrote: > > >>> > > >>> To François's point about code coverage for new code, I think this > > makes > > >> a > > >>> lot of sense wrt large features (like the current work on > > >> 8457/12229/9754). > > >>> It's much simpler to (mentally, at least) isolate those changed > > sections > > >>> and it'll show up better in a code coverage report. With small > patches, > > >>> that might be harder to achieve - however, as the patch should come > > with > > >>> *some* tests (unless it's a truly trivial patch), it might just work > > >> itself > > >>> out. > > >>> > > >>> On Fri, Mar 17, 2017 at 11:19 AM, Jason Brown <jasedbr...@gmail.com> > > >>> wrote: > > >>> > > >>>> As someone who spent a lot of time looking at the singletons topic > in > > >> the > > >>>> past, Blake brings a great perspective here. Figuring out and > > >>> communicating > > >>>> how best to test with the system we have (and of course > incrementally > > >>>> making that system easier to work with/test) seems like an > achievable > > >>> goal. > > >>>> > > >>>> On Fri, Mar 17, 2017 at 10:17 AM, Edward Capriolo < > > >> edlinuxg...@gmail.com > > >>>> > > >>>> wrote: > > >>>> > > >>>>> On Fri, Mar 17, 2017 at 12:33 PM, Blake Eggleston < > > >> beggles...@apple.com > > >>>> > > >>>>> wrote: > > >>>>> > > >>>>>> I think we’re getting a little ahead of ourselves talking about DI > > >>>>>> frameworks. Before that even becomes something worth talking > about, > > >>> we’d > > >>>>>> need to have made serious progress on un-spaghettifying Cassandra > in > > >>> the > > >>>>>> first place. It’s an extremely tall order. Adding a DI framework > > >> right > > >>>>> now > > >>>>>> would be like throwing gasoline on a raging tire fire. > > >>>>>> > > >>>>>> Removing singletons seems to come up every 6-12 months, and > usually > > >>>>>> abandoned once people figure out how difficult they are to remove > > >>>>> properly. > > >>>>>> I do think removing them *should* be a long term goal, but we > really > > >>>>> need > > >>>>>> something more immediately actionable. Otherwise, nothing’s going > to > > >>>>>> happen, and we’ll be having this discussion again in a year or so > > >> when > > >>>>>> everyone’s angry that Cassandra 5.0 still isn’t ready for > > >> production, > > >>> a > > >>>>>> year after it’s release. > > >>>>>> > > >>>>>> That said, the reason singletons regularly get brought up is > because > > >>>>> doing > > >>>>>> extensive testing of anything in Cassandra is pretty much > > >> impossible, > > >>>>> since > > >>>>>> the code is basically this big web of interconnected global state. > > >>>>> Testing > > >>>>>> anything in isolation can’t be done, which, for a distributed > > >>> database, > > >>>>> is > > >>>>>> crazy. It’s a chronic problem that handicaps our ability to > release > > >> a > > >>>>>> stable database. > > >>>>>> > > >>>>>> At this point, I think a more pragmatic approach would be to draft > > >> and > > >>>>>> enforce some coding standards that can be applied in day to day > > >>>>> development > > >>>>>> that drive incremental improvement of the testing and testability > of > > >>> the > > >>>>>> project. What should be tested, how it should be tested. How to > > >> write > > >>>>> new > > >>>>>> code that talks to the rest of Cassandra and is testable. How to > fix > > >>>>> bugs > > >>>>>> in old code in a way that’s testable. We should also have some > > >>>>> guidelines > > >>>>>> around refactoring the wildly untested sections, how to get > started, > > >>>>> what > > >>>>>> to do, what not to do, etc. > > >>>>>> > > >>>>>> Thoughts? > > >>>>> > > >>>>> > > >>>>> To make the conversation practical. There is one class I personally > > >>> really > > >>>>> want to refactor so it can be tested: > > >>>>> > > >>>>> https://github.com/apache/cassandra/blob/trunk/src/java/org/ > > >>>>> apache/cassandra/net/OutboundTcpConnection.java > > >>>>> > > >>>>> There is little coverage here. Questions like: > > >>>>> what errors cause the connection to restart? > > >>>>> when are undropable messages are dropped? > > >>>>> what happens when the queue fills up? > > >>>>> Infamous throw new AssertionError(ex); (which probably bubble up to > > >>>>> nowhere) > > >>>>> what does the COALESCED strategy do in case XYZ. > > >>>>> A nifty label (wow a label you just never see those much!) > > >>>>> outer: > > >>>>> while (!isStopped) > > >>>>> > > >>>>> Comments to jira's that probably are not explicitly tested: > > >>>>> // If we haven't retried this message yet, put it back on the queue > > to > > >>>>> retry after re-connecting. > > >>>>> // See CASSANDRA-5393 and CASSANDRA-12192. > > >>>>> > > >>>>> If I were to undertake this cleanup, would there actually be > support? > > >> IE > > >>>>> if > > >>>>> this going to turn into an "it aint broken. don't fix it thing" or > a > > >> "we > > >>>>> don't want to change stuff just to add tests" . Like will someone > > >> pledge > > >>>>> to > > >>>>> agree its kinda wonky and merge the effort in < 1 years time? > > >>>>> > > >>>> > > >>>> > > >>> > > >> > > >> So ...:) If open a ticket to refactor OutboundTcpConnection.java to do > > >> specific unit testing and possibly even pull things out to the point > > that I > > >> can actually open a socket and to an end to end test will you/anyone > > >> support that? (it sounds like your saying I must/should make a large > > >> feature to add a test) > > >> > > > > > > -- > Thank you & Best Regards, > --Simon (Qingcun) Zhou > Here is an example of a DYI mockito: https://issues.apache.org/jira/secure/attachment/12817379/12016-trunk.patch It reminds me of a saying I heard once: When you were a child you had a birthday party and a clown scared you. Now, you are all grown up and your have a kid who is having a party. You do not want to scare your kid, so you don't get a clown. Since you do not have a clown, you run around telling jokes, blowing up balloons, sitting on whoopee cushions, blowing Kazoo's. A balloon pops and a kid crys, then one point it occurs to you, "Holy crap! I am the f in clown!"