ARMmbed / htrun

MOVED: https://github.com/ARMmbed/mbed-os-tools (Flash, reset and run host supervised tests on mbed platforms)
8 stars 37 forks source link

disable timeout during consume_preamble_events #203

Closed alekla01 closed 6 years ago

alekla01 commented 6 years ago

This pr disables timeout in host_test_default during setup (allocation, flashing, etc..).

related: MBEDOSTEST-41

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.02%) to 43.743% when pulling 6dcd4b920b0d69ac75afe1a1dd7df3cfee3f11ff on alekla01:update_timeout into 51edce4f3e3f32b254678f4640c4ec30844c2a7a on ARMmbed:master.

jupe commented 6 years ago

Please add more description why this changes - what was the problem and how this tries to fix it.

alekla01 commented 6 years ago

master:

-> execute
|| parallel

0 sec: run_test  -> start process conn_process()
0 sec: run_test  -> while timeout_duration: handle events || conn_process -> preamble(allocate, flash, sync, etc..)
60 sec: run_test -> timeout, send __host_test_finished    || conn_process -> preamble
60 sec: run_test -> join(conn_process)                    || conn_process -> preamble
80 sec: run_test -> joined                                || conn_process -> preamble
90 sec: run_test -> joined                                || conn_process -> preamble done, ready to begin testing.
90 sec: run_test -> joined                                || conn_process -> oh, received __host_test_finished, well lets not run the tests then.
90 sec: run_test -> result=timeout                        || conn_process -> exit

solution:

-> execute
|| parallel

0 sec: run_test   -> start process conn_process()
0 sec: run_test   -> handle events                         || conn_process -> preamble(allocate, flash, sync, etc..)
60 sec: run_test  -> handle events                         || conn_process -> preamble
80 sec: run_test  -> handle events                         || conn_process -> preamble
90 sec: run_test  -> handle events                         || conn_process -> preamble done, ready to begin testing.
90 sec: run_test  -> timeout_duration=test_case_timeout    || conn_process -> begin_testing
90 sec: run_test  -> while timeout_duration: handle events || conn_process -> testing, send events
100 sec: run_test -> result=test_result                    || conn_process -> tests executed, exit

The solution is not ideal, but should work better than the current implementation.

theotherjimmy commented 6 years ago

@bridadan Does this seem like an okay fix to you. I'm not sure I'm wiling to drop a timeout...

bridadan commented 6 years ago

I don't think removing the timeout is the right fix. If the process hangs indefinitely it could seriously hurt CI processes.

From the JIRA issue you mentioned, I suggested just increasing the polling timeout (-P or --polling-timeout). Would that not be enough in this situation?