Initial unit tests for the discovery API.

Initial unit tests for the discovery API.

Includes a slight change to the behavior of chrome.discovery.suggest, documentation updated.

BUG=none
TEST=none


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=139273

[beaudoin, 2012-05-23 23:05:06]

Friendly ping! :)

[Aaron Boodman, 2012-05-28 18:32:12]

lgtm

http://codereview.chromium.org/10388192/diff/1/chrome/browser/extensions/api/...
File chrome/browser/extensions/api/discovery/discovery_api_unittest.cc (right):

http://codereview.chromium.org/10388192/diff/1/chrome/browser/extensions/api/...
chrome/browser/extensions/api/discovery/discovery_api_unittest.cc:87:
"[{\"linkUrl\": \"https://amazon.com/\", \"linkText\": \"Amazon\"}]");
The JSON schema compiler-generated structs will allow you to build these in a
strong typed way and then serialize them to JSON. It might read a little nicer.

I can't remember the methods but you should be able to find it.

[beaudoin, 2012-05-28 22:46:34]

I did not find any method that automatically generate JSON from the API function
parameters, but I've written 3 functions that do that, generating the
DictionaryValue for the struct from the JSON schema compiler-generated code. Let
me know if that's not what you had in mind.

http://codereview.chromium.org/10388192/diff/1/chrome/browser/extensions/api/...
File chrome/browser/extensions/api/discovery/discovery_api_unittest.cc (right):

http://codereview.chromium.org/10388192/diff/1/chrome/browser/extensions/api/...
chrome/browser/extensions/api/discovery/discovery_api_unittest.cc:87:
"[{\"linkUrl\": \"https://amazon.com/\", \"linkText\": \"Amazon\"}]");
On 2012/05/28 18:32:12, Aaron Boodman wrote:
> The JSON schema compiler-generated structs will allow you to build these in a
> strong typed way and then serialize them to JSON. It might read a little
nicer.
> 
> I can't remember the methods but you should be able to find it.

Done.

[commit-bot: I haz the power, 2012-05-28 22:46:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/beaudoin@chromium.org/10388192/5003

[commit-bot: I haz the power, 2012-05-29 00:27:41]

Change committed as 139273

[Aaron Boodman, 2012-05-29 01:00:05]

Oh, it looks like we don't generate params.ToValue(), we only do it for structs.
Looks like it should be easy to add:

http://code.google.com/searchframe#OAMlx_jo-ck/src/tools/json_schema_compiler...

[beaudoin1, 2012-05-29 15:16:17]

I will add them with my next pass at the extension.

One question though: are we worried about adding unused code to the
executable? Most of these ToValue() are only useful in tests. Should we
#ifdef guard them in some way or we don't really care about this?

Cheers,

    Philippe


On Mon May 28 21:00:05 GMT-400 2012, <aa@chromium.org> wrote:

> Oh, it looks like we don't generate params.ToValue(), we only do it for
> structs.
> Looks like it should be easy to add:
>
>
>
http://code.google.com/searchframe#OAMlx_jo-ck/src/tools/json_schema_compiler...
>
>
https://chromiumcodereview.appspot.com/10388192/<https://www.google.com/url?s...
>

[Aaron Boodman, 2012-05-29 20:40:19]

On Tue, May 29, 2012 at 8:16 AM, Philippe Beaudoin <beaudoin@google.com> wrote:
> I will add them with my next pass at the extension.
>
> One question though: are we worried about adding unused code to the
> executable? Most of these ToValue() are only useful in tests. Should we
> #ifdef guard them in some way or we don't really care about this?

That is a good point. I vaguely remember that we have the linker setup
so that such code will be removed, but I'm not sure. I guess it is not
something we should rely on.

And as far as I know, there is no compile-time symbol for test code.
And I don't see how one could work with the component build anyway.

So yeah, I guess the way you have it is better. I guess the "right
way" to do this is to generate the code only used by tests in separate
files and only include those files in the unit test binaries.

- a

[beaudoin1, 2012-05-29 23:03:56]

Interesting idea, some kind of generated api-specific testutil. I wonder if
struct's ToValue is relied upon anywhere in prod code. If not, it would be
a good place to trim. If yes, maybe we'll find some places using a crutch
to replace parameters ToValue similar to what I have in test code, so
generating them could be an opportunity for clean up.

I'll check all this next time I play around in that code.

Cheers,

   Philippe




On Tue May 29 16:39:48 GMT-400 2012, Aaron Boodman <aa@chromium.org> wrote:

> On Tue, May 29, 2012 at 8:16 AM, Philippe Beaudoin <beaudoin@google.com>
> wrote:
> > I will add them with my next pass at the extension.
> >
> > One question though: are we worried about adding unused code to the
> > executable? Most of these ToValue() are only useful in tests. Should we
> > #ifdef guard them in some way or we don't really care about this?
>
> That is a good point. I vaguely remember that we have the linker setup
> so that such code will be removed, but I'm not sure. I guess it is not
> something we should rely on.
>
> And as far as I know, there is no compile-time symbol for test code.
> And I don't see how one could work with the component build anyway.
>
> So yeah, I guess the way you have it is better. I guess the "right
> way" to do this is to generate the code only used by tests in separate
> files and only include those files in the unit test binaries.
>
> - a
>

[Aaron Boodman, 2012-05-30 18:17:48]

On Tue, May 29, 2012 at 1:39 PM, Aaron Boodman <aa@chromium.org> wrote:
> And as far as I know, there is no compile-time symbol for test code.
> And I don't see how one could work with the component build anyway.

I found out today that there is a UNIT_TEST macro...

- a