+1, agree to keep test running.
Regards,
----
Klaus Ma (马达), PMP® | http://www.cguru.net
> On Aug 15, 2015, at 14:45, Bernd Mathiske <be...@mesosphere.io> wrote:
>
> +1, but…
>
> If we are going to touch all our tests, then IMHO while at it we might as
> well make a jump forward to something better than the current local return
> void to abort tests in macros.
>
> If we used exceptions instead, it should be easy to catch those in a wrapper
> somewhere in the test class and then we could install some
> test-class-specific or test-superclass-specific or test-specific, but NOT
> macro-specific (!), extra code that prints out extra diagnostic info iff the
> test has failed. For example, it can dump the contents of the sandbox.
>
> If you really don’t like exceptions (?), we could make tests return a value
> instead of void and make the macros indicate that the test failed that way.
> Then we could also have a failure hook in the wrapper. (This is somewhat
> inferior, because it still does not support using the macros in nested
> methods/functions with a different return type. So I prefer exceptions. Not
> saying I want them everywhere in Mesos. Just in tests! 2c)
>
> I have started to modify one individual AWAIT macro for the above purpose,
> because I need more info when an unreproducable flaky fetcher cache tests
> failure happens. I need the extra info dump then and there when it happens,
> because I cannot get to it it later. The problem with this approach is that
> it instruments only one macro. Here is what it looks like (ugly!):
>
> #define AWAIT_READY_FOR_WITH_FAILURE_HOOK(actual, duration, onFailureHook) \
> GTEST_PRED_FORMAT2_( \
> AwaitAssertReady, \
> actual, \
> duration, \
> return [=](const char *message) { \
> onFailureHook(); \
> GTEST_FATAL_FAILURE_(message); \
> })
>
> #define AWAIT_READY_WITH_FAILURE_HOOK(actual, onFailureHook) \
> AWAIT_READY_FOR_WITH_FAILURE_HOOK(actual, Seconds(15), onFailureHook)
>
> Then I use it this way:
>
> AWAIT_READY_WITH_FAILURE_HOOK(
> someFuture,
> [=]() { logSandbox(); }); // action on failure by timeout
>
> Before you ask, this approach does not work, i.e. the output on failure does
> not happen:
>
> AWAIT_READY(someFuture
> .onFailed([=]() { logSandbox(); });
>
> ---
>
> Always printing out all the info would IMHO be prohibitively much.
>
> An alternative would be to have two log files, one at a lower log level for
> success only and one with the highest log level, which buildbot prints only
> in case something bad happens. etc. We could overwrite the latter after each
> individual test. This approach would mean extra work on the logging system.
>
> Opinions?
>
> Of course, Paul’s proposal should be tackled in any case. :-)
>
> Bernd
>
>> On Aug 15, 2015, at 1:24 AM, Marco Massenzio <ma...@mesosphere.io> wrote:
>>
>> +1
>>
>> *Marco Massenzio*
>>
>> *Distributed Systems Engineerhttp://codetrips.com <http://codetrips.com>*
>>
>> On Fri, Aug 14, 2015 at 3:46 PM, Paul Brett <pbr...@twitter.com.invalid>
>> wrote:
>>
>>> We are currently using the Google log CHECK macros (CHECK_SOME,
>>> CHECK_NOTNULL etc) in the test harness, usually to verify test setup. When
>>> these checks fail, it causes the test harness to abort rather than simply
>>> move onto the next test. The abort prevents any subsequent tests from
>>> running, hiding errors and preventing the generation of the XML test
>>> report.
>>>
>>> I would like to propose that we eliminate the use of CHECK in the test
>>> harness and replace it with the appropriate Google test macros to fail the
>>> test case.
>>> I
>>> am not proposing that we change the use of CHECK outside the test harness
>>> (although CHECK calls in master and slave can also kill the test harness).
>>>
>>> For void functions, CHECK can
>>> easily
>>> be replaced with the corresponding ASSERT equivalent.
>>>
>>> For non-void function, ASSERT cannot be used because it does not return the
>>> correct data type and hence we need to use a combination of ADD_FAILURE()
>>> and return.
>>>
>>> For example:
>>>
>>> CHECK(foo)
>>>
>>> would become:
>>>
>>> if(!foo) {
>>> ADD_FAILURE();
>>> return anything;
>>> }
>>>
>>> If there is general agreement, I will raise tickets to update the Mesos
>>> testing patterns document and each of the test cases.
>>>
>>> Thanks
>>>
>>>
>>> -- Paul Brett
>>>
>
