1081065 - Fix broken test coverage

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::TestAgent, defect)

Description

[Ricky Chien [:rickychien] (inactive)]

Current test coverage has been broken. Maybe it's because we upgraded libraries so that cause fails.

Comment 1

[Ricky Chien [:rickychien] (inactive)]

Coverage was broken after we updated test-agent.js in Bug 1068236 (see commit [1]). Switch to legacy test-agent.js fix this problem.

[1] https://github.com/RickyChien/gaia/commit/e6207091cf045581be27a7f4adc3f66e601ceca8#diff-72d099c1bf82904fc475cfdf1b4f0d01

I'm going to take a look into it and submit a patch!

Comment 2

[Ricky Chien [:rickychien] (inactive)]

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.

Comment 3

[Julien Wajsberg [:julienw]]

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?

Comment 4

[Ricky Chien [:rickychien] (inactive)]

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.

Comment 5

[Ricky Chien [:rickychien] (inactive)]

Attachment: Gaia PR

I found two weird errors:

1. blanket report doesn't show since it couldn't get its current script at [1]. 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[2] 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!

[1] https://github.com/mozilla-b2g/gaia/blob/f3cf5187520cdae91cb2b20583efc9fc294432bc/dev_apps/test-agent/common/vendor/blanket/blanket.js#L6696-L6698
[2] https://github.com/mozilla-b2g/gaia/pull/25286/files

Comment 6

[Ricky Chien [:rickychien] (inactive)]

Updated my patch!

Even though I haven't resolved issue yet, most of the tests were able to pass through replacing Promise.all() with [1] 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).

[1] https://github.com/mozilla-b2g/gaia/pull/25286/files#diff-bc377f19df7784162a70aba0e6306bdaR3034

Comment 7

[Ricky Chien [:rickychien] (inactive)]

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

Comment 8

[Ricky Chien [:rickychien] (inactive)]

Hi, I've leaved a comment on Github. :)

Comment 9

[Ricky Chien [:rickychien] (inactive)]

Attachment: Js-test-agent PR

Comment 10

[Ricky Chien [:rickychien] (inactive)]

Attachment: Gaia-node-modules

Comment 11

[Julien Wajsberg [:julienw]]

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 12

[Julien Wajsberg [:julienw]]

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!

Comment 13

[Ricky Chien [:rickychien] (inactive)]

Thank you Julien!

Gaia-try status:

https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=5f83864bb3b9

waiting for green.

Comment 14

[Ricky Chien [:rickychien] (inactive)]

Gaia-try is green after re-trigger Gb with an intermittent fail task in Gij!

Comment 15

[Ricky Chien [:rickychien] (inactive)]

Merged.

https://github.com/mozilla-b2g/gaia/commit/396035af948d3e9c601f0a0ddac583f6cf1d9750

https://github.com/mozilla-b2g/js-test-agent/commit/ab4c17b166805fbe01e98a7ea7aa3fff52568ca1

https://github.com/mozilla-b2g/gaia-node-modules/commit/f04758d8e20eda53b06cf4a999e3ae498166ed39

Comment 16

[Ricky Chien [:rickychien] (inactive)]

Merged update gaia_node_modules.revision

https://github.com/mozilla-b2g/gaia/commit/7afd1d7ce55fa80a865cccf306865dc1f406fad9