search

thaliproject / CI

CI project for testing mobile devices
MIT License
2 stars 3 forks source link

CI lies about tests' success #81

Closed evabishchevich closed 7 years ago

evabishchevich commented 7 years ago

Look at the https://github.com/thaliproject/Thali_CordovaPlugin/pull/1183. There is my comment after

Test 86930490d7f4847(d7f4847) has successfully finished without an error

See https://github.com/ThaliTester/TestResults/tree/86930490d7f4847_Android_changes_to_superbranch_evabishchevich/ for the logs

If you look at the logs on the devices you see that there are failures. But CI said that tests successfully finished

yaronyg commented 7 years ago

It's back!

czyzm commented 7 years ago

Issue 1: Native UT are failing but CI reports success and is executing node tests on devices which failed UT. If the native UT fail, the device should be disqualified from the node tests (the same way as the device with unsupported hardware). We haven't seen this before because node tests were not executed and we reported success or failure immediately after native tests (with regards to issue thaliproject/CI#80). After enabling node tests and removing the immediate failure report of native tests from UnitTest_app.js this should be handling later on by usage of global flag nativeUTFailed which is set in UnitTest_app.js. It seems that after changes in TestServer the code for handling this flag is no longer in place. Related commit: https://github.com/thaliproject/Thali_CordovaPlugin/commit/5fdde63aec681b345e6717872e27cc5f0fb4abeb

To handle this: Add support for nativeUTFailed flag similar like for not supported hardware. Here we need to also ensure that when device is disqualified because of failing native UT it adds into log '_TEST_LOGGER:[PROCESS_ON_EXIT_FAILED]_' so that CI knows that this device failed.

Issue 2: There is a timeout during one of node tests but all the devices are logging _TEST_LOGGER:[PROCESS_ON_EXIT_SUCCESS]_\ saying that all tests passed. Looking at the UnitTestFramework.js function startTests I see that in the promise chain we have finally where we call device.complete(). So when any test fails we are logging that it failed (in .catch) but then we go to .finally and call device.complete(). If it failed in a way that it didn't add _TEST_LOGGER:[PROCESS_ON_EXIT_FAILED]_\ to the logs the device.complete() call will result in adding _TEST_LOGGER:[PROCESS_ON_EXIT_SUCCESS]_\ and CI will handle this as a passed run.

To handle this: Similar like for issue 1 we need to make sure that in case node test fails the device is adding to the logs _TEST_LOGGER:[PROCESS_ON_EXIT_FAILED]_. I would do it in the mentioned .catch - inform CoordinatedClient that we have error.

czyzm commented 7 years ago

Fixed with https://github.com/thaliproject/Thali_CordovaPlugin/pull/1223