Moving test_api to api/test .

Moving test_api to api/test .


BUG=101244
TEST=N/A, only moving code, no change in functionality


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

[vabr (Chromium), 2012-07-10 16:14:40]

Hi Yoyo,

Could you please review this?

Thanks,
Vaclav

[Yoyo Zhou, 2012-07-10 19:00:10]

LGTM, just nits.

https://chromiumcodereview.appspot.com/10690113/diff/7002/chrome/browser/exte...
File chrome/browser/extensions/api/test/test_api.cc (right):

https://chromiumcodereview.appspot.com/10690113/diff/7002/chrome/browser/exte...
chrome/browser/extensions/api/test/test_api.cc:20: namespace extensions {
You could also put this at 34, so the anonymous namespace isn't in the
extensions namespace. I don't have a strong opinion on this; perhaps look at
some other files to see what the convention is.

https://chromiumcodereview.appspot.com/10690113/diff/7002/chrome/browser/exte...
File chrome/browser/extensions/api/test/test_api.h (right):

https://chromiumcodereview.appspot.com/10690113/diff/7002/chrome/browser/exte...
chrome/browser/extensions/api/test/test_api.h:26: class
ExtensionTestPassFunction : public TestExtensionFunction {
Now that these are in the namespace, you don't need the Extension part of the
function names anymore. In fact, I would probably rename this one to
TestNotifyPassFunction to actually match the name in js. Same with the other
functions below.

https://chromiumcodereview.appspot.com/10690113/diff/7002/chrome/browser/exte...
chrome/browser/extensions/api/test/test_api.h:59: class
ExtensionTestQuotaResetFunction : public TestExtensionFunction {
This one seems like it should be called TestResetQuotaFunction.

https://chromiumcodereview.appspot.com/10690113/diff/7002/chrome/browser/exte...
chrome/browser/extensions/api/test/test_api.h:136: } // namespace extensions
nit: two spaces before the //

https://chromiumcodereview.appspot.com/10690113/diff/7002/chrome/browser/exte...
File chrome/browser/extensions/extension_test_message_listener.h (right):

https://chromiumcodereview.appspot.com/10690113/diff/7002/chrome/browser/exte...
chrome/browser/extensions/extension_test_message_listener.h:17: } // namespace
extensions
nit: probably don't need this comment since it's so short.

https://chromiumcodereview.appspot.com/10690113/diff/7002/chrome/browser/exte...
File chrome/browser/extensions/extensions_quota_service.h (right):

https://chromiumcodereview.appspot.com/10690113/diff/7002/chrome/browser/exte...
chrome/browser/extensions/extensions_quota_service.h:32: } // namespace
extensions
ditto

[vabr (Chromium), 2012-07-11 10:01:25]

Thanks, Yoyo,
I addressed your comments, and I'm putting this into CQ.

Cheers,

Vaclav

http://codereview.chromium.org/10690113/diff/7002/chrome/browser/extensions/a...
File chrome/browser/extensions/api/test/test_api.cc (right):

http://codereview.chromium.org/10690113/diff/7002/chrome/browser/extensions/a...
chrome/browser/extensions/api/test/test_api.cc:20: namespace extensions {
On 2012/07/10 19:00:11, Yoyo Zhou wrote:
> You could also put this at 34, so the anonymous namespace isn't in the
> extensions namespace. I don't have a strong opinion on this; perhaps look at
> some other files to see what the convention is.

Done.
Both occur in the api/ subdirectory, with the one you suggest being more
frequent.

http://codereview.chromium.org/10690113/diff/7002/chrome/browser/extensions/a...
File chrome/browser/extensions/api/test/test_api.h (right):

http://codereview.chromium.org/10690113/diff/7002/chrome/browser/extensions/a...
chrome/browser/extensions/api/test/test_api.h:26: class
ExtensionTestPassFunction : public TestExtensionFunction {
On 2012/07/10 19:00:11, Yoyo Zhou wrote:
> Now that these are in the namespace, you don't need the Extension part of the
> function names anymore. In fact, I would probably rename this one to
> TestNotifyPassFunction to actually match the name in js. Same with the other
> functions below.

Done.

http://codereview.chromium.org/10690113/diff/7002/chrome/browser/extensions/a...
chrome/browser/extensions/api/test/test_api.h:59: class
ExtensionTestQuotaResetFunction : public TestExtensionFunction {
On 2012/07/10 19:00:11, Yoyo Zhou wrote:
> This one seems like it should be called TestResetQuotaFunction.

Done.

http://codereview.chromium.org/10690113/diff/7002/chrome/browser/extensions/a...
chrome/browser/extensions/api/test/test_api.h:136: } // namespace extensions
On 2012/07/10 19:00:11, Yoyo Zhou wrote:
> nit: two spaces before the //

Done.

http://codereview.chromium.org/10690113/diff/7002/chrome/browser/extensions/e...
File chrome/browser/extensions/extension_test_message_listener.h (right):

http://codereview.chromium.org/10690113/diff/7002/chrome/browser/extensions/e...
chrome/browser/extensions/extension_test_message_listener.h:17: } // namespace
extensions
On 2012/07/10 19:00:11, Yoyo Zhou wrote:
> nit: probably don't need this comment since it's so short.

Done. The examples in the Chromium style guide confirm you are right.

http://codereview.chromium.org/10690113/diff/7002/chrome/browser/extensions/e...
File chrome/browser/extensions/extensions_quota_service.h (right):

http://codereview.chromium.org/10690113/diff/7002/chrome/browser/extensions/e...
chrome/browser/extensions/extensions_quota_service.h:32: } // namespace
extensions
On 2012/07/10 19:00:11, Yoyo Zhou wrote:
> ditto

Done. Also moved the forward decl. after those non-nested, to match the style
guide.

[commit-bot: I haz the power, 2012-07-11 10:01:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vabr@chromium.org/10690113/6016

[commit-bot: I haz the power, 2012-07-11 10:01:46]

Failed to apply patch for
chrome/browser/extensions/extension_function_registry.cc:
While running patch -p1 --forward --force;
patching file chrome/browser/extensions/extension_function_registry.cc
Hunk #1 FAILED at 44.
Hunk #2 succeeded at 233 with fuzz 1 (offset -4 lines).
1 out of 2 hunks FAILED -- saving rejects to file
chrome/browser/extensions/extension_function_registry.cc.rej

[commit-bot: I haz the power, 2012-07-11 10:07:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vabr@chromium.org/10690113/8002

[commit-bot: I haz the power, 2012-07-11 10:31:25]

Try job failure for 10690113-8002 (retry) on linux_chromeos for step "compile"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2012-07-11 11:11:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vabr@chromium.org/10690113/21

[commit-bot: I haz the power, 2012-07-11 12:33:37]

Change committed as 146107