Closed Bug 851443 Opened 11 years ago Closed 11 years ago

Create tests for richgrid binding

Categories

(Firefox for Metro Graveyard :: Tests, defect)

x86_64
Windows 8.1
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 22

People

(Reporter: sfoster, Assigned: sfoster)

References

Details

Attachments

(1 file, 1 obsolete file)

We rely on the richgrid and richgriditem bindings for all the tiles on the firefox start screen. The implementation of the various tile lists/grids is adding to and changing these bindings and what we expect of them. A set of chrome tests would help head off regressions and make explicit what the bindings are currently relied upon to do.
Blocks: 831918
This covers most of the richgrid and richgriditem functionality actually in 
use - with the significant exception of arrangeItems which I've left as a todo. 

I found and fixed a couple of issues - mostly around the single/multiple modes and the meaning of select vs. selection. My hope is the tests will spell out what these things are known to do so we can quickly spot regressions and/or know what to change as the bindings continue to evolve.
 
I make use of a new stubMethod test helper for some of the tests, which is working out nicely. Seems like this might/ought to exist somewhere already though?  

The tests all pass for me - let me know if you spot any failures or bogus/misleading assertions.
Attachment #725618 - Flags: review?(mbrubeck)
Assignee: nobody → sfoster
Attachment #725618 - Attachment is obsolete: true
Attachment #725618 - Flags: review?(mbrubeck)
Comment on attachment 725625 [details] [diff] [review]
New stubMethod helper and tests for most of the richgrid / richgriditem functionality

arrangeItems left as a todo. Also I've not covered the contextual actions stuff. Otherwise should be complete?
Attachment #725625 - Flags: review?(mbrubeck) → review?(jmathies)
Comment on attachment 725625 [details] [diff] [review]
New stubMethod helper and tests for most of the richgrid / richgriditem functionality

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

::: browser/metro/base/tests/browser_tiles.js
@@ +23,5 @@
> +
> +      is(grid.children.length, 1, "#grid1 has a single item");
> +      is(grid.children[0].control, grid, "#grid1 item's control points back at #grid1'");
> +    } catch(e) {
> +      info(this.desc + ": Caught exception: " + e.message);

Do we need this? I've added these myself and mbrubeck suggested I take them out such that unexpected exceptions cause failures. Ditto on all the others.
Attachment #725625 - Flags: review?(jmathies) → review+
Maybe not. I find at least during test implementation that it helps a lot as you get a more useful stack trace. Also/besides, tests that rely on previous tests to to have run smell bad. Maybe I can just re-throw from the catch block.
I removed the try/catch around each test and pushed to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/196693568165
https://hg.mozilla.org/mozilla-central/rev/196693568165
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 22
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: