Add Feature tag to all Payments tests

Add Feature tag to all Payments tests

This will allow users to exclusively run or ignore
all Payment tests at once.

BUG=642079
Committed: https://crrev.com/3bc619f82fb23fdaf8d9b0f82e05f00a23b7670e
Cr-Commit-Position: refs/heads/master@{#415086}

[kraush, 2016-08-17 23:11:10]

kraush@amazon.com changed reviewers:
+ miguelg@chromium.org

[kraush, 2016-08-17 23:11:10]

Hi Miguel,

Can you take a look at this change to introduce Feature tags for Payments tests?

Thanks! :)
Holger

[kraush, 2016-08-29 14:46:33]

kraush@amazon.com changed reviewers:
+ dfalcantara@chromium.org

[kraush, 2016-08-29 14:46:34]

No response from Miguel in a while (also pinged him via email) :(
Adding dfalcantara as a reviewer instead. (Which might also be more appropriate
in the first place, looking at git log of the payments directory)

[gone, 2016-08-29 17:29:50]

dfalcantara@chromium.org changed reviewers:
+ rouslan@chromium.org
- miguelg@chromium.org

[gone, 2016-08-29 17:29:51]

Deferring to rouslan@ to see if this is useful since they're mostly his tests.

[please use gerrit instead, 2016-08-29 17:40:18]

This is not useful. You can run these tests as follows:

ninja -Cout/gn chrome_public_test_apk_incremental &&
out/gn/bin/run_chrome_public_test_apk_incremental -vvv -f
org.chromium.chrome.browser.payments.*

Adding 120 lines of code to save 30 characters on command line seems wasteful.

I'm sorry about the wasted effort here. To avoid this in the future, first
please file a bug in UI>Browser>Autofill>Payments or Blink>Payments component.
We can discuss the best approach on the bug tracker and then one of us can fix
the issue that we know needs fixing.

[please use gerrit instead, 2016-08-29 17:40:24]

Description was changed from

==========
Add Feature tag to all Payments tests

This will allow users to exclusively run or ignore
all Payment tests at once.

BUG=
==========

to

==========
Add Feature tag to all Payments tests

This will allow users to exclusively run or ignore
all Payment tests at once.

BUG=
==========

[kraush, 2016-08-29 18:31:50]

On 2016/08/29 17:40:18, rouslan wrote:
> This is not useful. You can run these tests as follows:
> 
> ninja -Cout/gn chrome_public_test_apk_incremental &&
> out/gn/bin/run_chrome_public_test_apk_incremental -vvv -f
> org.chromium.chrome.browser.payments.*
> 
> Adding 120 lines of code to save 30 characters on command line seems wasteful.
> 
> I'm sorry about the wasted effort here. To avoid this in the future, first
> please file a bug in UI>Browser>Autofill>Payments or Blink>Payments component.
> We can discuss the best approach on the bug tracker and then one of us can fix
> the issue that we know needs fixing.

Is there a way to suppress them all at once?
The @Feature tag is also used in other test suites to suppress a multitude of
tests with -E
(This is actually our main use case why we want this tag)

[please use gerrit instead, 2016-08-29 19:18:40]

On 2016/08/29 18:31:50, kraush wrote:
> On 2016/08/29 17:40:18, rouslan wrote:
> > This is not useful. You can run these tests as follows:
> > 
> > ninja -Cout/gn chrome_public_test_apk_incremental &&
> > out/gn/bin/run_chrome_public_test_apk_incremental -vvv -f
> > org.chromium.chrome.browser.payments.*
> > 
> > Adding 120 lines of code to save 30 characters on command line seems
wasteful.
> > 
> > I'm sorry about the wasted effort here. To avoid this in the future, first
> > please file a bug in UI>Browser>Autofill>Payments or Blink>Payments
component.
> > We can discuss the best approach on the bug tracker and then one of us can
fix
> > the issue that we know needs fixing.
> 
> Is there a way to suppress them all at once?
> The @Feature tag is also used in other test suites to suppress a multitude of
> tests with -E
> (This is actually our main use case why we want this tag)

My testing shows that "-E Payments" does not filter out any tests. Is that
feature still functional? 

In either case, it seems that "-E" is the only way to exclude a group of tests.
If you need this, then it's OK to land this patch. However, please do file a bug
first to make this work easier to track.

[kraush, 2016-08-29 19:21:03]

On 2016/08/29 19:18:40, rouslan wrote:
> On 2016/08/29 18:31:50, kraush wrote:
> > On 2016/08/29 17:40:18, rouslan wrote:
> > > This is not useful. You can run these tests as follows:
> > > 
> > > ninja -Cout/gn chrome_public_test_apk_incremental &&
> > > out/gn/bin/run_chrome_public_test_apk_incremental -vvv -f
> > > org.chromium.chrome.browser.payments.*
> > > 
> > > Adding 120 lines of code to save 30 characters on command line seems
> wasteful.
> > > 
> > > I'm sorry about the wasted effort here. To avoid this in the future, first
> > > please file a bug in UI>Browser>Autofill>Payments or Blink>Payments
> component.
> > > We can discuss the best approach on the bug tracker and then one of us can
> fix
> > > the issue that we know needs fixing.
> > 
> > Is there a way to suppress them all at once?
> > The @Feature tag is also used in other test suites to suppress a multitude
of
> > tests with -E
> > (This is actually our main use case why we want this tag)
> 
> My testing shows that "-E Payments" does not filter out any tests. Is that
> feature still functional? 
> 
> In either case, it seems that "-E" is the only way to exclude a group of
tests.
> If you need this, then it's OK to land this patch. However, please do file a
bug
> first to make this work easier to track.

Gotcha, thanks, I'll file a bug! :)
"-E Feature=Payments" will, after this change, suppress all those tests.

[kraush, 2016-08-29 19:23:43]

Description was changed from

==========
Add Feature tag to all Payments tests

This will allow users to exclusively run or ignore
all Payment tests at once.

BUG=
==========

to

==========
Add Feature tag to all Payments tests

This will allow users to exclusively run or ignore
all Payment tests at once.

BUG=642079
==========

[kraush, 2016-08-29 19:25:03]

> > My testing shows that "-E Payments" does not filter out any tests. Is that
> > feature still functional? 
> > 
> > In either case, it seems that "-E" is the only way to exclude a group of
> tests.
> > If you need this, then it's OK to land this patch. However, please do file a
> bug
> > first to make this work easier to track.
> 
> Gotcha, thanks, I'll file a bug! :)
> "-E Feature=Payments" will, after this change, suppress all those tests.

https://bugs.chromium.org/p/chromium/issues/detail?id=642079
Please note: As a non-Googler, I don't think I'm able to move issues to any
folder or give them any tags. (Please let me know if that's not the case and I'd
be happy to adjust the issue)
Sorry for the inconvenience :(

[please use gerrit instead, 2016-08-29 19:27:57]

lgtm

[gone, 2016-08-29 20:02:47]

OWNERS lgtm

[kraush, 2016-08-29 20:06:35]

Thanks rouslan and dfalcantara! :)
I'll rebase on master and make sure this still covers all tests before
committing.
Will update with a patch set 2 if there are any additional tests.
I assume your lgtm is still good even if there are a few more test methods by
now. If you'd rather re-review, please let me know

Thanks! :)
Holger

[kraush, 2016-08-30 00:22:28]

On 2016/08/29 20:06:35, kraush wrote:
> Thanks rouslan and dfalcantara! :)
> I'll rebase on master and make sure this still covers all tests before
> committing.
> Will update with a patch set 2 if there are any additional tests.
> I assume your lgtm is still good even if there are a few more test methods by
> now. If you'd rather re-review, please let me know
> 
> Thanks! :)
> Holger

Rebased, rebuilt, and this still covers 100% of tests. Committing.

[kraush, 2016-08-30 00:22:33]

The CQ bit was checked by kraush@amazon.com

[kraush, 2016-08-30 00:48:11]

The CQ bit was unchecked by kraush@amazon.com

[kraush, 2016-08-30 00:48:12]

The CQ bit was checked by kraush@amazon.com

[commit-bot: I haz the power, 2016-08-30 00:49:20]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-30 04:20:10]

Description was changed from

==========
Add Feature tag to all Payments tests

This will allow users to exclusively run or ignore
all Payment tests at once.

BUG=642079
==========

to

==========
Add Feature tag to all Payments tests

This will allow users to exclusively run or ignore
all Payment tests at once.

BUG=642079
==========

[commit-bot: I haz the power, 2016-08-30 04:20:11]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-08-30 04:23:17]

Description was changed from

==========
Add Feature tag to all Payments tests

This will allow users to exclusively run or ignore
all Payment tests at once.

BUG=642079
==========

to

==========
Add Feature tag to all Payments tests

This will allow users to exclusively run or ignore
all Payment tests at once.

BUG=642079

Committed: https://crrev.com/3bc619f82fb23fdaf8d9b0f82e05f00a23b7670e
Cr-Commit-Position: refs/heads/master@{#415086}
==========

[commit-bot: I haz the power, 2016-08-30 04:23:18]

Patchset 1 (id:??) landed as
https://crrev.com/3bc619f82fb23fdaf8d9b0f82e05f00a23b7670e
Cr-Commit-Position: refs/heads/master@{#415086}