442778 - test for bug 419731 failing on qm-win2k3-pgo01 (VM, PGO)

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Dietrich Ayala (:dietrich)]

Comment 1

[Dietrich Ayala (:dietrich)]

looks like this is similar to bug 427142.

Comment 2

[Mike Beltzner [:beltzner, not reading bugmail]]

Log snippet:

Build Error Summary

../../../../_tests/xpcshell-simple/test_places/unit/test_419731.js: FAIL
make[3]: *** [check] Error 1
make[2]: *** [check] Error 2
make[1]: *** [check] Error 2
make: *** [check] Error 2

Build Error Log

 Skipping 25597 Lines...

../../../../_tests/xpcshell-simple/test_places/bookmarks/test_393498.js: PASS
../../../../_tests/xpcshell-simple/test_places/bookmarks/test_395101.js: PASS
../../../../_tests/xpcshell-simple/test_places/bookmarks/test_395593.js: PASS
../../../../_tests/xpcshell-simple/test_places/bookmarks/test_405938_restore_queries.js: PASS
../../../../_tests/xpcshell-simple/test_places/bookmarks/test_417228-exclude-from-backup.js: PASS
../../../../_tests/xpcshell-simple/test_places/bookmarks/test_417228-other-roots.js: PASS
../../../../_tests/xpcshell-simple/test_places/bookmarks/test_423515_forceCopyShortcuts.js: PASS
../../../../_tests/xpcshell-simple/test_places/bookmarks/test_424958-json-quoted-folders.js: PASS
../../../../_tests/xpcshell-simple/test_places/bookmarks/test_bookmarks.js: PASS
../../../../_tests/xpcshell-simple/test_places/bookmarks/test_livemarks.js: PASS
../../../../_tests/xpcshell-simple/test_places/bookmarks/test_removeItem.js: PASS
../../../../_tests/xpcshell-simple/test_places/bookmarks/test_savedsearches.js: PASS
../../../../_tests/xpcshell-simple/test_places/queries/test_abstime-annotation-domain.js: PASS
../../../../_tests/xpcshell-simple/test_places/queries/test_abstime-annotation-uri.js: PASS
../../../../_tests/xpcshell-simple/test_places/queries/test_searchterms-uri.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_000_frecency.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_317472.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_331487.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_385397.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_399264_query_to_string.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_399264_string_to_query.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_399266.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_399606.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_402799.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_404630.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_405497.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_408221.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_413784.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_415460.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_415757.js: PASS
NEXT ERROR ../../../../_tests/xpcshell-simple/test_places/unit/test_419731.js: FAIL
../../../../_tests/xpcshell-simple/test_places/unit/test_419731.js.log:
>>>>>>>
*** test pending
*** exiting
*** CHECK FAILED: new title 1 == new title 2

 Skipping 5 Lines...

2147500036
*** FAIL ***
Exception: [Exception... "Component returned failure code: 0x80520015 (NS_ERROR_FILE_ACCESS_DENIED) [nsIFile.remove]"  nsresult: "0x80520015 (NS_ERROR_FILE_ACCESS_DENIED)"  location: "JS frame :: ../../../../_tests/xpcshell-simple/test_places/unit/head_bookmarks.js :: clearDB :: line 99"  data: no]
<<<<<<<
../../../../_tests/xpcshell-simple/test_places/unit/test_421180.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_425563.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_429505_remove_shortcuts.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_433525_hasChildren_crash.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_adaptive.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_annotations.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_browserhistory.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_download_history.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_dynamic_containers.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_exclude_livemarks.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_expiration.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_favicons.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_frecency.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_history.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_history_autocomplete_tags.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_history_sidebar.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_isvisited.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_markpageas.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_multi_queries.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_multi_word_tags.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_nsINavHistoryViewer.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_placeURIs.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_resolveNullBookmarkTitles.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_result_sort.js: PASS
../../../../_tests/xpcshell-simple/test_places/unit/test_tagging.js: PASS

It seems to be permissions based, which makes me wonder:

 - does nuking the profile help?
 - are we supremely confident that this test could not have regressed since the last time the box was green?
 - if so, should we disable the test and get on with our lives?

Disabling tests makes me nervous, but we need to continue with PGO coverage, so assuming nothing else touches that places code, we should be OK?

What does that test test, anyway?

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

Oh, it tests bug 419731, duh.

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

Dietrich: what do you need from that box to evaluate this? We can attach it to this bug if you tell us:

17:34 < shaver> dietrich: what data do you need from that system to diagnose 
                the problem?
17:35 < dietrich> shaver: the places.sqlite file

Anything else?

Coop / someone: can we get the places.sqlite file placed somewhere accessible?

Comment 5

[Dietrich Ayala (:dietrich)]

I think we are hitting the same problem as in bug 427142:

Here's what the test does:
* creates two bookmarks, both with the same URI
* that URI is tagged
* the database is queried for that tagged URI
* one of the bookmark titles is modified

Expected result: the title of the query-result-node for the tagged URI should be the title of the most recently modified bookmark for that URI

Actual: the title of the node is not the expected title

In a bug last year (which I cannot find), as well as bug 427142, the conclusion was that PR_Now() in some environments can return the same value across multiple calls.

This could lead to a scenario where the database might not return the title of the most recently modified bookmark.

Getting the places.sqlite file from the test box *may* be able to corroborate this. The test harness usually removes the places.sqlite file after each test is run, creating a fresh file for each test. However, in this instance, the log shows a permissions problem attempting to do so. If this is the case, then there's a possiblity that the places.sqlite will in fact have the data from this particular test. I've requested that file in bug 442777.

Comment 6

[Dietrich Ayala (:dietrich)]

(In reply to comment #5)
> Getting the places.sqlite file from the test box *may* be able to corroborate
> this. The test harness usually removes the places.sqlite file after each test
> is run, creating a fresh file for each test. However, in this instance, the log
> shows a permissions problem attempting to do so. If this is the case, then
> there's a possiblity that the places.sqlite will in fact have the data from
> this particular test. I've requested that file in bug 442777.
> 

No such luck. The file only contained the default bookmarks.

Comment 7

[Dietrich Ayala (:dietrich)]

I advocate rigging or disabling the test, so that 3.0.1 work can continue.

1. The probability of this occurring in a real-world scenario is low: the user needs two bookmarks with the same URI, and would need to change *both* titles within a few microseconds time span. In a VM with PGO turned on.

2. The outcome of failure of that code is that a tagged URI has a title of one of the user's bookmarks which is possibly not the most recently modified bookmark for that URI. I can live with that.

Comment 8

[Dietrich Ayala (:dietrich)]

We should also have someone familiar with PR_Now() look deeper into how it behaves in that specific test environment.

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

Dietrich: can you whip up that patch for consideration?

Comment 10

[Dietrich Ayala (:dietrich)]

Attachment: disable the test for now

Comment 11

[Rob Campbell [:rc] (:robcee)]

just looking at the tinderbox waterfall for today, it looks like this machine's been orange several times for different reasons. Honestly, since this box landed in March or April, it's been sporadic, probably due to load on the VMHost it lives on. I wouldn't necessarily hold the release based on this orange, though allowing a few (long) cycles to pass to get a better look at the failure patterns would be a good idea. If the failures are suitably random, you should proceed with the building.

my 2 bits.

Comment 12

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 327491 [details] [diff] [review]
disable the test for now

r+a=beltzner

Comment 13

[Mike Beltzner [:beltzner, not reading bugmail]]

Let's disable the test until we're done baking, then do the analysis that Rob proposes in comment 11; Dietrich, can you file a follow up bug against the Build & Release team with that and suggest analysis mechanisms?

Comment 14

[Dietrich Ayala (:dietrich)]

Checking in toolkit/components/places/tests/unit/test_419731.js;
/cvsroot/mozilla/toolkit/components/places/tests/unit/test_419731.js,v  <--  test_419731.js
new revision: 1.3; previous revision: 1.2
done

Comment 15

[Samuel Sidler (old account; do not CC)]

This machine never went green. What else can we do here?

Comment 16

[Dietrich Ayala (:dietrich)]

My checkin did resolve the Places unit test problem. The new failure is in mochitest, looks like a focus or window size issue.

This bug is specifically about test_419731.js, so I recommend opening a new bug for the new test failures.

Or we could stop wasting time trying to coddle this box's eccentricities, and take it offline. See bug 442819.

Marking this bug fixed, since the reported issue is fixed.

Comment 17

[Tony Chung [:tchung]]

test disabled for 3.0.1

Comment 18

[Gervase Markham [:gerv]]

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv