<div dir="ltr"><div>I'll skip straight to the conclusions in the paper:</div><div><br></div><div>> • Keep regression tests around for up to a year — but most of</div><div>those will be system-level tests rather than unit tests.</div>
<div><br></div><div>I would say most of our tests qualify as regression tests, and they are system-level (they start a full cache manager, with a JGroups channel and everything).</div><div>But how would we know that they haven't failed in a year? I mean we can find which tests never failed in CI, but we can't find which tests never failed on any dev's machine (thus helping that dev find a problem in his code).</div>
<div><div><br></div></div><div>> • Keep unit tests that test key algorithms for which there is a</div><div>broad, formal, independent oracle of correctness, and for</div><div>which there is ascribable business value.</div>
<div>> • Except for the preceding case, if X has business value and you</div><div>can text X with either a system test or a unit test, use a system</div><div>test — context is everything.</div><div><br></div><div><div>
We have about 100 tests in the "unit" group, some of which should really be in the "functional" group (e.g. BaseStoreTest).</div><div>It shouldn't take long to walk over those and delete some that may be tested better by system tests, but it won't have a visible effect on the test suite run time either.</div>
</div><div><br></div><div>> • Design a test with more care than you design the code.<br></div><div><br></div><div>I admit that when reviewing PRs I pay less attention to the tests than to the production code. I will try to change that :)</div>
<div><br></div><div>> • Turn most unit tests into assertions.</div><div><br></div><div>I imagine some kinds of tests can indeed be replaced with assertions in production code, but again we don't have so many unit tests.</div>
<div><br></div><div>> • Throw away tests that haven’t failed in a year.</div><div>> • Testing can’t replace good development: a high test failure<br></div><div>rate suggests you should shorten development intervals,</div>
<div>perhaps radically, and make sure your architecture and design</div><div>regimens have teeth</div><div>> • If you find that individual functions being tested are trivial,</div><div>double-check the way you incentivize developers’</div>
<div>performance. Rewarding coverage or other meaningless</div><div>metrics can lead to rapid architecture decay.</div><div>> • Be humble about what tests can achieve. Tests don’t improve</div><div>quality: developers do.</div>
<div><br></div><div><div style="color:rgb(21,21,21);font-family:Menlo,'Bitstream Vera Sans Mono','Ubuntu Mono','Courier New',Courier,monospace;font-size:12px;line-height:16.799999237060547px">
<div style="white-space:pre-wrap;color:rgb(63,63,63)"><span><br></span></div></div></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Jun 10, 2014 at 6:23 PM, Galder Zamarreño <span dir="ltr"><<a href="mailto:galder@redhat.com" target="_blank">galder@redhat.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Just recently I read this: <a href="http://www.rbcs-us.com/documents/Why-Most-Unit-Testing-is-Waste.pdf" target="_blank">http://www.rbcs-us.com/documents/Why-Most-Unit-Testing-is-Waste.pdf</a><br>
<br>
Food for thought, and maybe we should consider doing for next major. Maybe if we’re moving to JUnit we could look at this in detail too?<br>
<br>
Cheers,<br>
<div><div class="h5"><br>
On 09 Jun 2014, at 23:06, Sanne Grinovero <<a href="mailto:sanne@infinispan.org">sanne@infinispan.org</a>> wrote:<br>
<br>
> On 9 June 2014 14:42, Dan Berindei <<a href="mailto:dan.berindei@gmail.com">dan.berindei@gmail.com</a>> wrote:<br>
>><br>
>><br>
>> On Mon, Jun 9, 2014 at 12:43 PM, Sanne Grinovero <<a href="mailto:sanne@infinispan.org">sanne@infinispan.org</a>><br>
>> wrote:<br>
>>><br>
>>> Hi Dan!<br>
>>><br>
>>> The reason is that I'm making substantial API changes in the Query<br>
>>> module, and I've lost count on how many other modules and integration<br>
>>> tests depend on it: I need to run all the testsuite to evaluate where<br>
>>> I'm heading.. I don't need it just as last touches but continually, to<br>
>>> be able to make good choices while work is in progress.<br>
>><br>
>><br>
>> Wouldn't "mvn install -DskipTests" be enough for testing dependencies?<br>
>> You could then use "mvn surefire:test -pl query,lucene-directory" to run<br>
>> just the tests you're interested in.<br>
>> I have a script that does just that - even for core, I don't want to think<br>
>> about whether I changed something in commons or not, but I almost never want<br>
>> to run the commons tests :)<br>
><br>
> I need to run all modules, and AFAIK there is no way to run the tests<br>
> on all modules except one (unless you list all of them explicitly..<br>
> good luck with that).<br>
><br>
> What I did to go ahead in this ball of mud is to patch the core<br>
> pom.xml to add "skipTests" option to the surefire configuration, at<br>
> least I could test my stuff.<br>
><br>
><br>
>>> Not only I'm changing API but also substantial changes in the<br>
>>> dependency tree.. without a working testsuite I can't make progress.<br>
>>> I'm working around it by deleting core in a temporary commit... :-/<br>
>>> (and even so the suite takes more than an hour ??!)<br>
>>><br>
>><br>
>> 1 hour just for the core, or for everything? I used to get about 1h for<br>
>> everything, if just the core takes that long to fail it's definitely a<br>
>> problem. There was also a bit of a slowdown after the JGroups 3.5.0.Beta7<br>
>> upgrade (ISPN-4355), but that should be fixed now.<br>
><br>
> It's 1h and 20 minutes now to test all modules. This took 12 minutes a<br>
> couple of months ago. My opinion is that even 12 minutes is too slow,<br>
> I would consider it acceptable if we where running some soak tests but<br>
> as far as I know we're just load testing the testing framework.<br>
><br>
> Sanne<br>
><br>
>><br>
>> I was also thinking of moving the failsafe plugin to the "extras" profile,<br>
>> so that we can avoid running the server integration tests in dev builds. But<br>
>> disabling the extras profile also disables the bundling for OSGi, so perhaps<br>
>> a separate profile would be better.<br>
>><br>
>> Dan<br>
>><br>
>><br>
>>><br>
>>> I'll test your PRs ASAP, thanks a lot.<br>
>>><br>
>>> Cheers,<br>
>>> Sanne<br>
>>><br>
>>><br>
>>> On 9 June 2014 10:19, Dan Berindei <<a href="mailto:dan.berindei@gmail.com">dan.berindei@gmail.com</a>> wrote:<br>
>>>><br>
>>>><br>
>>>><br>
>>>> On Fri, Jun 6, 2014 at 4:31 PM, Sanne Grinovero <<a href="mailto:sanne@infinispan.org">sanne@infinispan.org</a>><br>
>>>> wrote:<br>
>>>>><br>
>>>>> I'm having several failures, these are blocking our progress on Query.<br>
>>>>><br>
>>>>> Should I disable them all?<br>
>>>><br>
>>>><br>
>>>> You could disable them, but I'm not quite sure how that would help the<br>
>>>> query<br>
>>>> module... surely you don't need to run the core tests every time you<br>
>>>> modify<br>
>>>> something in query?<br>
>>>><br>
>>>>><br>
>>>>><br>
>>>>> Sample output of three different runs:<br>
>>>>><br>
>>>>> Failed tests:<br>
>>>>> ThreadLocalLeakTest.testCheckThreadLocalLeaks:87 IllegalState Thread<br>
>>>>> locals st...<br>
>>>><br>
>>>><br>
>>>> I couldn't reproduce this failure on my machine, but I modified<br>
>>>> ThreadLocalLeakTest to ignore that particular thread-local:<br>
>>>> <a href="https://github.com/infinispan/infinispan/pull/2614" target="_blank">https://github.com/infinispan/infinispan/pull/2614</a><br>
>>>> You're the only one that reported seeing it, so please test the PR on<br>
>>>> your<br>
>>>> machine and integrate it.<br>
>>>><br>
>>>><br>
>>>>><br>
>>>>><br>
>>>>><br>
>>>>> StateTransferOverwriteTest>BaseTxStateTransferOverwriteTest.testStateTransferInBetweenPrepareCommitWithPutIfAbsent:104->BaseTxStateTransferOverwriteTest.doStateTransferInBetweenPrepareCommit:265<br>
>>>>> » Runtime<br>
>>>>> Tests run: 5430, Failures: 2, Errors: 0, Skipped: 0<br>
>>>><br>
>>>><br>
>>>> I've created <a href="https://issues.jboss.org/browse/ISPN-4368" target="_blank">https://issues.jboss.org/browse/ISPN-4368</a> for Will to look<br>
>>>> into, I'm not sure we need the "placeholder" key that is causing the<br>
>>>> random<br>
>>>> failures.<br>
>>>><br>
>>>>><br>
>>>>><br>
>>>>> Failed tests:<br>
>>>>> ThreadLocalLeakTest.testCheckThreadLocalLeaks:87 IllegalState Thread<br>
>>>>> locals st...<br>
>>>>><br>
>>>>><br>
>>>>> StateTransferOverwriteTest>BaseTxStateTransferOverwriteTest.testStateTransferInBetweenPrepareCommitWithPutIfAbsent:104->BaseTxStateTransferOverwriteTest.doStateTransferInBetweenPrepareCommit:265<br>
>>>>> » Runtime<br>
>>>>><br>
>>>>><br>
>>>>> L1StateTransferOverwriteTest>BaseTxStateTransferOverwriteTest.testStateTransferInBetweenPrepareCommitWithPut:84->BaseTxStateTransferOverwriteTest.doStateTransferInBetweenPrepareCommit:265<br>
>>>>> » Runtime<br>
>>>><br>
>>>><br>
>>>> The L1StateTransferOverwriteTest failure seems to have the same cause as<br>
>>>> StateTransferOverwriteTest failure.<br>
>>>><br>
>>>>><br>
>>>>> Tests run: 5430, Failures: 3, Errors: 0, Skipped: 0<br>
>>>>><br>
>>>>> Failed tests:<br>
>>>>> ThreadLocalLeakTest.testCheckThreadLocalLeaks:87 IllegalState Thread<br>
>>>>> locals st...<br>
>>>>><br>
>>>>><br>
>>>>> CacheNotifierImplInitialTransferDistTest.testCreateAfterIterationBeganAndSegmentNotCompleteValueOwnerClustered:611->testIterationBeganAndSegmentNotComplete:510<br>
>>>>> expected [11] but found [6]<br>
>>>><br>
>>>><br>
>>>> I've seen this a couple times on my machine as well, I've created<br>
>>>> <a href="https://issues.jboss.org/browse/ISPN-4370" target="_blank">https://issues.jboss.org/browse/ISPN-4370</a><br>
>>>><br>
>>>>><br>
>>>>> Tests run: 5430, Failures: 2, Errors: 0, Skipped: 0<br>
>>>><br>
>>>><br>
>>>><br>
>>>> I'd let Will investigate ISPN-4368 and ISPN-4370 for a bit before<br>
>>>> disabling<br>
>>>> those tests - he may be able to issue a PR for them quite quickly.<br>
>>>><br>
>>>> Still, just because I don't like disabling tests it doesn't mean I don't<br>
>>>> appreciate your stability reports - keep 'em coming!<br>
>>>><br>
>>>><br>
>>>> Cheers<br>
>>>> Dan<br>
>>>><br>
>>>><br>
>>>><br>
>>>> _______________________________________________<br>
>>>> infinispan-dev mailing list<br>
>>>> <a href="mailto:infinispan-dev@lists.jboss.org">infinispan-dev@lists.jboss.org</a><br>
>>>> <a href="https://lists.jboss.org/mailman/listinfo/infinispan-dev" target="_blank">https://lists.jboss.org/mailman/listinfo/infinispan-dev</a><br>
>>><br>
>>> _______________________________________________<br>
>>> infinispan-dev mailing list<br>
>>> <a href="mailto:infinispan-dev@lists.jboss.org">infinispan-dev@lists.jboss.org</a><br>
>>> <a href="https://lists.jboss.org/mailman/listinfo/infinispan-dev" target="_blank">https://lists.jboss.org/mailman/listinfo/infinispan-dev</a><br>
>><br>
>><br>
>><br>
>> _______________________________________________<br>
>> infinispan-dev mailing list<br>
>> <a href="mailto:infinispan-dev@lists.jboss.org">infinispan-dev@lists.jboss.org</a><br>
>> <a href="https://lists.jboss.org/mailman/listinfo/infinispan-dev" target="_blank">https://lists.jboss.org/mailman/listinfo/infinispan-dev</a><br>
><br>
> _______________________________________________<br>
> infinispan-dev mailing list<br>
> <a href="mailto:infinispan-dev@lists.jboss.org">infinispan-dev@lists.jboss.org</a><br>
> <a href="https://lists.jboss.org/mailman/listinfo/infinispan-dev" target="_blank">https://lists.jboss.org/mailman/listinfo/infinispan-dev</a><br>
<br>
<br>
</div></div>--<br>
Galder Zamarreño<br>
<a href="mailto:galder@redhat.com">galder@redhat.com</a><br>
<a href="http://twitter.com/galderz" target="_blank">twitter.com/galderz</a><br>
<div class="HOEnZb"><div class="h5"><br>
<br>
_______________________________________________<br>
infinispan-dev mailing list<br>
<a href="mailto:infinispan-dev@lists.jboss.org">infinispan-dev@lists.jboss.org</a><br>
<a href="https://lists.jboss.org/mailman/listinfo/infinispan-dev" target="_blank">https://lists.jboss.org/mailman/listinfo/infinispan-dev</a><br>
</div></div></blockquote></div><br></div>