Closed Bug 975792 Opened 10 years ago Closed 10 years ago

ContentProvider tests don't clean up their cursors

Categories

(Firefox for Android Graveyard :: Testing, defect)

All
Android
defect
Not set
normal

Tracking

(firefox30 fixed)

RESOLVED FIXED
Firefox 30
Tracking Status
firefox30 --- fixed

People

(Reporter: rnewman, Assigned: rnewman)

References

Details

Attachments

(1 file, 1 obsolete file)

Spun off from other work. This causes horrible logspew.
Attachment #8380309 - Flags: review?(nalexander)
Fix some borked indentation.
Attachment #8380310 - Flags: review?(nalexander)
Attachment #8380309 - Attachment is obsolete: true
Attachment #8380309 - Flags: review?(nalexander)
Comment on attachment 8380310 [details] [diff] [review]
ContentProvider tests don't clean up their cursors. v2

Review of attachment 8380310 [details] [diff] [review]:
-----------------------------------------------------------------

Not running tests locally for the loss!

::: mobile/android/base/tests/testBrowserProvider.java
@@ +1836,5 @@
>              ensureOnlyChangeNotifiedStartsWith(BrowserContract.History.CONTENT_URI, "bulkInsert");
>          }
>      }
> +
> +    void assertIsAndClose(Cursor c, int expectedCount, String message) {

|assertCountIsAndClose|?
Attachment #8380310 - Flags: review?(nalexander) → review+
Pushed with name change, and added docstring and `private`:

https://hg.mozilla.org/integration/fx-team/rev/49a1923db9be
Follow-up, 'cos the favicon check that I re-enabled fails on Tinderbox but not on my device.

https://hg.mozilla.org/integration/fx-team/rev/1f6450d61d3f
Depends on: 975969
https://hg.mozilla.org/mozilla-central/rev/49a1923db9be
https://hg.mozilla.org/mozilla-central/rev/1f6450d61d3f
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
Backed out for too frequent robocop failures (bug 975969 and more).

https://hg.mozilla.org/mozilla-central/rev/1a219f500336
https://hg.mozilla.org/mozilla-central/rev/1422dfcd7fd8
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Good guy rnewman leaves a note in the Sheriff's etherpad, gets backed out
Note that the test previously did not check what it was supposed to check. So this push:

https://tbpl.mozilla.org/?tree=Try&rev=e4e2e255d5ab

just logs any irregularities, rather than actually asserting them. If it's something obvious, I'll fix; otherwise, I'll just reland it without the checks that fail.
Relanded with the failing checks commented out.

https://hg.mozilla.org/integration/fx-team/rev/9122f3be8c3a
Status: REOPENED → ASSIGNED
Yeah, when your note is in effect saying "even if I landed 50% orange I don't want to be backed out" you don't want to put it in that etherpad, you want to put it... um, in... uh..., I think the only place you could put that note would be in the second line of the commit message for the cset that completely disables the test that you were otherwise going to make 50% orange.

BTW, so far on the reland, https://tbpl.mozilla.org/php/getParsedLog.php?id=35167881&tree=Fx-Team
Huh, build didn't fail locally.

Also, this would be way less painful if TBPL would load.
Apparently CursorDumper is only available at build time, not at run time.

https://hg.mozilla.org/integration/fx-team/rev/efa293b98893
Depends on: 976482
Relanded after three green Try rc1s, without the foolish expectation that our core database code works.

https://hg.mozilla.org/integration/fx-team/rev/4881a4e25a3e
https://hg.mozilla.org/mozilla-central/rev/4881a4e25a3e
Status: ASSIGNED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
(In reply to Richard Newman [:rnewman] from comment #9)
> Note that the test previously did not check what it was supposed to check.
> So this push:
> 
> https://tbpl.mozilla.org/?tree=Try&rev=e4e2e255d5ab
> 
> just logs any irregularities, rather than actually asserting them. If it's
> something obvious, I'll fix; otherwise, I'll just reland it without the
> checks that fail.

BTW, this is filed as bug 949048.
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: