489405 - Test case not running as expected

Status: RESOLVED WORKSFORME

(Core :: DOM: Geolocation, defect)

Description

[Murali Nandigama [:murali]]

I and Doug Turner were investigating code coverage on a specific file using a specific test case

http://mxr.mozilla.org/mozilla-central/source/dom/tests/mochitest/geolocation/geolocation.html?force=1

This test case when executed, is supposed to touch the following code piece in the following file src/dom/base/nsGlobalWindow.cpp function 
nsNavigator::nsIDOMNavigatorGeolocation which is the last function call in the file.

However, while in mochitest logs, we see this test case as being executed, the code block is not touched.

However, in the location bar if I type "javascript:navigator.geolocation" the code in the function is executed.

So, there is something going on that requires further investigation. Since this function is not being called in mochitest run, the coverage information related to the file in geolocation is shown as 12% which is wrong.

For a demo, please come by to my desk for a demo.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

For the test, presumably we're using TestGeolocationProvider.js, which replaces most of the code in dom/src/geolocation/ and wouldn't be tracked, since it's JS. That shouldn't affect nsNavigator code, though, which should be running either way.

Comment 2

[cmtalbert]

The code coverage does track JS files, but like Gavin, I think this is probably due to some difference in the test geolocation provider's registration process.  For mochitests we use a test geolocation provider, but when you run it outside the product, we run the real geolocation provider.

Comment 3

[Murali Nandigama [:murali]]

http://mxr.mozilla.org/mozilla-central/source/dom/base/nsGlobalWindow.cpp Lines 9537 to 9548 should be executed if the test is run as expected

Comment 4

[Murali Nandigama [:murali]]

So, how do we make sure that developers test cases are executed as they intended ?
Doug: You take over from here

Comment 5

[Doug Turner (:dougt)]

clint and I have a theory.  Murali, is it possible for you to edit a specific file and rerun the tests?  If so, we can confirm that we have identified the problem.

Comment 6

[Doug Turner (:dougt)]

@Murali:  if the answer is yes, just comment out the contents of this function:

http://mxr.mozilla.org/mozilla-central/source/dom/tests/mochitest/geolocation/geolocation_common.js#5

The mochitest may fail, but there should be a notable change in test coverage.

Comment 7

[Murali Nandigama [:murali]]

That is doable. Let me re run the test after the change

Comment 8

[Murali Nandigama [:murali]]

Your plan works. If i comment out the http://mxr.mozilla.org/mozilla-central/source/dom/tests/mochitest/geolocation/geolocation_common.js#5 and rerun the mochitest using the following command

 python runtests.py --console-level=INFO --autorun --test-path=dom/tests/mochitest/geolocation


I get coverage on the function @

http://mxr.mozilla.org/mozilla-central/source/dom/base/nsGlobalWindow.cpp Lines
9537 to 9548

Comment 9

[Doug Turner (:dougt)]

thanks Murali.  we found the problem, and your stuff found it.  I guess I owe you that drink.

Comment 10

[Doug Turner (:dougt)]

the problem is that we have two geolocation providers: a test one, and a real one.  We have some code (the stuff that I asked Murali to comment out) to ensure that we use the test component.  That is failing and causing the mochitest to return early.

Fixing 488542 will clean up this test component (so that it never is in dist/bin/components), and allow us to remove this failing code.  There may be other work arounds.

Comment 11

[cmtalbert]

I'm not convinced that the test component registration stuff (though it should probably still be cleaned up) is at fault here. In fact, I'm not certain what is going on at all with this.  Here's what Murali and I tried this afternoon:

1 Murali verified that commenting out the enablePrivilege('UniversalXPConnect'); line caused the lines to be covered in his code coverage analysis.  Of course, that also caused the tests to fail.
2 We sat down and put the enablePrivilege stuff back in.  And I put a try/catch block around the entire contents of the ensure_geolocationProvider() function
 ** We verified that the tests indeed passed and *no errors were caught* by my try/catch.  That's the first sign something weird was happening.
** We analyzed the code coverage data and there was again no coverage.
** I thought that we should negate the fact that code coverage stuff might be lying to us and put a break point on the uncovered lines in nsGlobalWindow.cpp and ran the tests again.  The lines were hit.
3 We thought that was odd, so we rolled back the changes in test 2 and ran with the proper setting and the gdb breakpoint.  Again the lines were hit.
** At this point we looked at the gcov data generated from this test and saw that the lines are reporting as covered.

So, this really doesn't tell us much but it does confuse the issue quite a bit.

Out of all this I have three possible outcomes:
* There is still something wrong with the registration
* There is something wrong with the coverage data making a transition from gcov to lcov html output
* There is a mysterious heisenbug affecting our results

Murali is going to see what tonight's releng coverage results say.
I think we should do the following tests and try to determine what area the bug is in.
= See if the Rel Eng builds from tonight still report no coverage here =
* if so, proceed
* if not, chalk it up to weird ass bad luck?
= On an Instrumented debug build, no changes to source =
* Run the geolocation tests with the gdb breakpoint set  to see if it is hit
* verify that gcov supports what gdb is telling us
* verify that lcov results support what gcov and gdb are telling us
* this will tell us if we have a problem in the actual code coverage results

= On an instrumented debug build, no changes to source =
* Run geolocation tests without gdb breakpoint and see what gcov says
* See what lcov says and see if those two agree.
* This is to rule out any oddity of the fact we're debugging in gdb - I rather think this test is moot, but for completeness' sake, it's best not to make any assumptions

= On an instrumented debug build (if everything above doesn't show any problems) =
* Change ensure_geolocationProvider to call the default provider from the system (i.e. not the test provider)
* Run the same two tests above with and without gdb and see if we get different code coverage numbers.

Determine if we are coming off trace when we call into the C++ function.  I know that they have just finished wrapped natives, and I know that you can't do code coverage on traced calls.  It might be that the first C++ call after trace gets messed up, but subsequent c++ calls are ok.  I also think this is highly unlikely, but it's the best hypothesis I can come up with if everything above is working out ok.

That's my thought on next steps.  I'll put this in the bug too.

Comment 12

[Doug Turner (:dougt)]

Murali, any update?

Comment 13

[Doug Turner (:dougt)]

wtf -- now i see we are at 88.4 %

Comment 14

[Murali Nandigama [:murali]]

Don't look at those numbers. they are done using the js file hack yesterday. commenting line #5

I am running a build with your patch as we speak.

Comment 15

[Doug Turner (:dougt)]

the patch that you are running with; i do not think that will work actually.

Comment 16

[Murali Nandigama [:murali]]

Do you mean the patch from https://bugzilla.mozilla.org/show_bug.cgi?id=488542

Comment 17

[Doug Turner (:dougt)]

murali, is this still an issue?

Comment 18

[Murali Nandigama [:murali]]

Not a bug any more

Comment 19

[Jason Bassford]

It's not at all clear what fixed this - the comments are contradictory.

Comment 20

[Murali Nandigama [:murali]]

I guess the work done by Doug in comments 9 and 10 should answer your question. 

The bottomline is that we have coverage numbers for that code part.