[infinispan-dev] Example server config tests disabled?

[Sanne Grinovero]

On 9 July 2014 16:32, Mircea Markus <mmarkus at redhat.com> wrote:
>
> On Jul 7, 2014, at 10:37, Sanne Grinovero <sanne at infinispan.org> wrote:
>
>>> looks like all example config tests were marked as "Unstable" and hence
>>> disabled. I see a note "See ISPN-4026" in ExampleConfigsIT.java and it
>>> leads to https://issues.jboss.org/browse/ISPN-4026. This would be fine
>>> if not all tests for all example configs were disabled.
>>> As a result, tests for the example config for rolling upgrades did not
>>> run and this issue was ignored:
>>> https://issues.jboss.org/browse/ISPN-4026  This looks like a critical
>>> issue to me.
>>>
>>> PS: I will always wonder how we can be disabling tests in this way.
>>
>> +1
>> I don't understand how we can keep pushing things which break stuff by
>> disabling tests rather than rolling back broken patches.
>>
>> There are situations in which a test is badly designed and it should
>> be disabled, but I've had lots of tests in Query disabled because of
>> stuff not working as it should in core.. stuff which used to work.
>> In such cases one shouldn't disable the test but roll-back the changes
>> which broke it, and think better about the "fixes" rather than move
>> responsibility to others to figure it out eventually.
>
> Tests should not be disabled in order to be fixed at a future point in time, but to be investigated in background and allow others to integrate PRs into upstream. Just disabling tests and not looking into them is pointless. We'll reenable all the tests before going beta.

Cool, but also it's important to make a distinction between
 A) tests which are not correct or not deterministic enough to be
trusted (we need to get rid of these as it makes the whole process
very painful)
 B) correct tests which suddenly start to fail

In case of B you don't want to investigate after several weeks, it's
much easier if you're not allowed to merge a PR until at least you
know what relation there is between the changes and the failing test.
If there is none, then you're in case A, right? But if there is a
relation, then you *might* decide that you opt to disable the test and
go ahead, but at least you make it a responsible decision and know
exactly what is going on, and also makes it easier to estimate further
blockers.

Tests should never be disabled for the convenience of investigating
"tomorrow", only when you can clearly say it's not a deterministic
one.

Sanne

>
> Cheers,
> --
> Mircea Markus
> Infinispan lead (www.infinispan.org)
>
>
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev