Closed Bug 1081065 Opened 7 years ago Closed 7 years ago
Fix broken test coverage
Current test coverage has been broken. Maybe it's because we upgraded libraries so that cause fails.
Assignee: nobody → ricky060709
Status: NEW → ASSIGNED
Coverage was broken after we updated test-agent.js in Bug 1068236 (see commit ). Switch to legacy test-agent.js fix this problem.  https://github.com/RickyChien/gaia/commit/e6207091cf045581be27a7f4adc3f66e601ceca8#diff-72d099c1bf82904fc475cfdf1b4f0d01 I'm going to take a look into it and submit a patch!
Hi Julien, Do you have any idea? I saw test-agent.js has been replaced by Promise style so that cause coverage failures. At present, I ran Bluetooth tests and it arose sinon.stub errors in pair_manager_test.js. These sinon.stub are located on setup() so I try to output console log and see what happened. And then, incorrect times of console message appeared.
I spent ~2 hours on this but I don't have a lot of ideas :/ What's weird is that we have an error in the setup() but the test still runs, I think it shouldn't (unless the Blanket driver ignores the failures ?). The error looks like the sinon_helper teardown function is not run (the teardown function should restore sinon's spies and stubs), but I added a log in it, and it's correctly run. So I really don't know what's happening. I don't remember enough how coverage works to help more right now :/ What are your own ideas on this?
I can't catch it too. Blanket.js will execute code instrumentation before mocha running tests. Instrumentation modifies all source scripts but doesn't impact original logic so that coverage tool can calculate tracking data. I'm sure with promise, test-agent can load blanket.js properly before running all tests. But I couldn't figure out what's happened in run-time. I still suspect the root cause in these promise code.
I found two weird errors: 1. blanket report doesn't show since it couldn't get its current script at . It seems unstable to use old approach to find current script out, however, I solved by replacing with document.currentScript. 2. I guess that there are some conflicts between the procedure of worker.loader.done and blanket. Removing worker.loader.done() from test-agent.js can pass bluetooth tests along with a proper coverage report. Now the remained issue 2 has to be analyzed. Julien, do you get any idea in such conflicts? Thanks!  https://github.com/mozilla-b2g/gaia/blob/f3cf5187520cdae91cb2b20583efc9fc294432bc/dev_apps/test-agent/common/vendor/blanket/blanket.js#L6696-L6698  https://github.com/mozilla-b2g/gaia/pull/25286/files
Updated my patch! Even though I haven't resolved issue yet, most of the tests were able to pass through replacing Promise.all() with  and some fails were 'lazyload' feature in blanket which was implemented by me. It's so hard to solve this bug since I'm suspicious of these setTimeout-like implementation existed in promise method. It probably has effect on blanket's 'lazyload' feature. I manage to catch up why my patch works. However, I haven't come out with ideas with it. I can pass most of the tests but only fail in the case which override testAgentRuntime.testLoader to its custom loader(i.e. calendar).  https://github.com/mozilla-b2g/gaia/pull/25286/files#diff-bc377f19df7784162a70aba0e6306bdaR3034
Updated PR again. I seemed to figure out what's going on here. Promise deferred mocha.run(), so blanket.js is unable to launch properly in some cases. My workaround is to customise blanket.js test runner so that can receive 'mocha-run' event after executing mocha.run(). Additionally, We have to ignore alameda.js due to unknown behaviours when require alameda.js in blanket.js
Attachment #8507320 - Flags: review?(felash)
Hi, I've leaved a comment on Github. :)
ok, I tried and it looks to work now. I had one failure in SMS but then I tried again and it worked. I'll look closer tomorrow.
Comment on attachment 8507320 [details] [review] Gaia PR let's land this I added 2 nits in the github PR for js-test-agent, so please fix them first. Don't hesitate to ask a review again if you'd like that I double check. Then land in the js-test-agent repository, publish test-agent to npm, rebuild node_modules, update node_modules in gaia in a PR (with "make update-common"), check that it's green in try, and land :) good luck!
Attachment #8507320 - Flags: review?(felash) → review+
Thank you Julien! Gaia-try status: https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=5f83864bb3b9 waiting for green.
Gaia-try is green after re-trigger Gb with an intermittent fail task in Gij!
Merged update gaia_node_modules.revision https://github.com/mozilla-b2g/gaia/commit/7afd1d7ce55fa80a865cccf306865dc1f406fad9
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.