search

thaliproject / CI

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

CI is incorrectly returning success when it should be returning failure #56

Closed yaronyg closed 8 years ago

yaronyg commented 8 years ago

I got the following message via Email from CI:

Test 79426650eeb77ae(eeb77ae) has successfully finished without an error See https://github.com/ThaliTester/TestResults/tree/79426650eeb77ae_Almost_to_the_top_of_the_Thali_stack_yaronyg/ for the logs

Which was both exciting and of course mystifying since it shouldn't be working. In fact when I groveled through the logs I saw that we are failing on 'Cannot find module '../thalilogger' but I'll file a separate bug on that.

This bug is - why the heck did I get a mail saying the tests passed when they didn't?

czyzm commented 8 years ago

This is related to the issue https://github.com/thaliproject/Thali_CordovaPlugin/issues/594 (App fails to load on some devices to error "No such native module"). The fix for this is already done with following PR: https://github.com/thaliproject/jxcore/pull/11

There was a code introduced to simply report success when app.js fails to load (so that we are not stopped with this jxcore issue): https://github.com/thaliproject/Thali_CordovaPlugin/commit/84971526dd7e936d975cf2ffc4779e45f307ba75

This is not CI issue but the Cordova plugin issue.

yaronyg commented 8 years ago

The code that reports success when app.js fails to load needs to be removed at this point. In fact, it really shouldn't have been there at all. Instead we should have removed the node tests until we were ready again. Please confirm that code is gone.

yaronyg commented 8 years ago

O.k. I get it. I filed https://github.com/thaliproject/Thali_CordovaPlugin/issues/842.

@enricogior depending on how many failures we see as a result of pulling the work around here for https://github.com/thaliproject/jxcore/pull/11 we may need to push out a release of JXCore-Cordova earlier than I thought in order to use a version of JXcore that fixes that problem. We'll see.