Some unit tests

Some unit tests.

I started to develop unit tests for merging cards, but then switched to other stuff. I don't want the added code to rotten on my machine, so I'm submitting it. Later, I'll add more tests.

BUG=164227
TEST=No

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=230194

[robliao, 2013-10-21 23:08:33]

https://codereview.chromium.org/32583004/diff/1/chrome/browser/resources/goog...
File chrome/browser/resources/google_now/background_unittest.gtestjs (right):

https://codereview.chromium.org/32583004/diff/1/chrome/browser/resources/goog...
chrome/browser/resources/google_now/background_unittest.gtestjs:325: // Tests
mergeAndShowNotificationCards function.
Discuss what is passed in and what's expected.

https://codereview.chromium.org/32583004/diff/1/chrome/browser/resources/goog...
chrome/browser/resources/google_now/background_unittest.gtestjs:348:
eqJSON({card1: {testValue: 'TEST CARD VALUE 1'}})),
Is card1 guaranteed to be the first thing considered? The ordering of fields
that point to non-primitive types is undefined.

http://www.ecma-international.org/ecma-262/5.1/#sec-12.6.4

[vadimt, 2013-10-22 00:57:07]

https://codereview.chromium.org/32583004/diff/1/chrome/browser/resources/goog...
File chrome/browser/resources/google_now/background_unittest.gtestjs (right):

https://codereview.chromium.org/32583004/diff/1/chrome/browser/resources/goog...
chrome/browser/resources/google_now/background_unittest.gtestjs:325: // Tests
mergeAndShowNotificationCards function.
On 2013/10/21 23:08:33, Robert Liao wrote:
> Discuss what is passed in and what's expected.

Done.

https://codereview.chromium.org/32583004/diff/1/chrome/browser/resources/goog...
chrome/browser/resources/google_now/background_unittest.gtestjs:348:
eqJSON({card1: {testValue: 'TEST CARD VALUE 1'}})),
On 2013/10/21 23:08:33, Robert Liao wrote:
> Is card1 guaranteed to be the first thing considered? The ordering of fields
> that point to non-primitive types is undefined.
> 
> http://www.ecma-international.org/ecma-262/5.1/#sec-12.6.4

This is how our JS implementation works.
I doubt that V8 will suddently change the implementation, since they realize how
many people they'll break.
And even if they do this, they'll likely give us an early notice, so that we'll
have time to modify the tests (or fight back).

Note that we also rely on eqJSON, which assumes certain order of filds in
JSON-filed objects, and this never broke so far, even though the tests we run
presumably thousands of times.

[robliao, 2013-10-22 16:52:14]

LGTM

https://codereview.chromium.org/32583004/diff/1/chrome/browser/resources/goog...
File chrome/browser/resources/google_now/background_unittest.gtestjs (right):

https://codereview.chromium.org/32583004/diff/1/chrome/browser/resources/goog...
chrome/browser/resources/google_now/background_unittest.gtestjs:348:
eqJSON({card1: {testValue: 'TEST CARD VALUE 1'}})),
I think v8 is one of those implementations where the order can change on you.

Either way, for this test, should be fine unless it starts flaking at us.
On 2013/10/22 00:57:07, vadimt wrote:
> On 2013/10/21 23:08:33, Robert Liao wrote:
> > Is card1 guaranteed to be the first thing considered? The ordering of fields
> > that point to non-primitive types is undefined.
> > 
> > http://www.ecma-international.org/ecma-262/5.1/#sec-12.6.4
> 
> This is how our JS implementation works.
> I doubt that V8 will suddently change the implementation, since they realize
how
> many people they'll break.
> And even if they do this, they'll likely give us an early notice, so that
we'll
> have time to modify the tests (or fight back).
> 
> Note that we also rely on eqJSON, which assumes certain order of filds in
> JSON-filed objects, and this never broke so far, even though the tests we run
> presumably thousands of times.

[skare_, 2013-10-22 17:14:36]

lgtm

[commit-bot: I haz the power, 2013-10-22 17:32:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vadimt@chromium.org/32583004/50001

[commit-bot: I haz the power, 2013-10-22 20:12:26]

Change committed as 230194

[Ryan Sleevi, 2013-10-22 21:10:30]

On 2013/10/22 20:12:26, I haz the power (commit-bot) wrote:
> Change committed as 230194

In the future, please make sure your commit notes are more descriptive. The
description of this commit is not at all clear (and I had to hop onto this CL to
find out what it was and changed)

[vadimt, 2013-10-22 21:53:48]

On 2013/10/22 21:10:30, Ryan Sleevi wrote:
> On 2013/10/22 20:12:26, I haz the power (commit-bot) wrote:
> > Change committed as 230194
> 
> In the future, please make sure your commit notes are more descriptive. The
> description of this commit is not at all clear (and I had to hop onto this CL
to
> find out what it was and changed)

Thanks for the comment!