Revert of Make the repeat button accessible by keyboard.

Revert of Make the repeat button accessible by keyboard. (patchset #7 id:120001 of https://codereview.chromium.org/2482063002/ )

Reason for revert:
Broke https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%281%29, more info in http://crbug.com/662829#c3.

Cheers,
today's sheriff

Original issue's description:
> Make the repeat button accessible by keyboard.
> As a known issue, duplicated ripple effect will be shown when clicking the icon (instead of keyboard).
>
> BUG=662829
> TEST=manual test by hitting [TAB] key several times
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
>
> Committed: https://crrev.com/b5275a7dcc189413073ea961c1eb7d3768ef35b8
> Cr-Commit-Position: refs/heads/master@{#430897}

TBR=fukino@chromium.org,oka@chromium.org,yamaguchi@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=662829
Committed: https://crrev.com/76e2c525c63fcb6b744b0e2bb73394f5b17da4c7
Cr-Commit-Position: refs/heads/master@{#430913}

[vabr (Chromium), 2016-11-09 12:03:31]

The CQ bit was checked by vabr@chromium.org

[vabr (Chromium), 2016-11-09 12:03:31]

Created Revert of Make the repeat button accessible by keyboard.

[commit-bot: I haz the power, 2016-11-09 12:03:38]

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-11-09 12:04:55]

Description was changed from

==========
Revert of Make the repeat button accessible by keyboard. (patchset #7 id:120001
of https://codereview.chromium.org/2482063002/ )

Reason for revert:
Broke
https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%...,
more info in http://crbug.com/662829#c3.

Cheers,
today's sheriff

Original issue's description:
> Make the repeat button accessible by keyboard.
> As a known issue, duplicated ripple effect will be shown when clicking the
icon (instead of keyboard).
> 
> BUG=662829
> TEST=manual test by hitting [TAB] key several times
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
> 
> Committed: https://crrev.com/b5275a7dcc189413073ea961c1eb7d3768ef35b8
> Cr-Commit-Position: refs/heads/master@{#430897}

TBR=fukino@chromium.org,oka@chromium.org,yamaguchi@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=662829
==========

to

==========
Revert of Make the repeat button accessible by keyboard. (patchset #7 id:120001
of https://codereview.chromium.org/2482063002/ )

Reason for revert:
Broke
https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%...,
more info in http://crbug.com/662829#c3.

Cheers,
today's sheriff

Original issue's description:
> Make the repeat button accessible by keyboard.
> As a known issue, duplicated ripple effect will be shown when clicking the
icon (instead of keyboard).
> 
> BUG=662829
> TEST=manual test by hitting [TAB] key several times
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
> 
> Committed: https://crrev.com/b5275a7dcc189413073ea961c1eb7d3768ef35b8
> Cr-Commit-Position: refs/heads/master@{#430897}

TBR=fukino@chromium.org,oka@chromium.org,yamaguchi@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=662829
==========

[commit-bot: I haz the power, 2016-11-09 12:04:57]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-11-09 12:06:39]

Description was changed from

==========
Revert of Make the repeat button accessible by keyboard. (patchset #7 id:120001
of https://codereview.chromium.org/2482063002/ )

Reason for revert:
Broke
https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%...,
more info in http://crbug.com/662829#c3.

Cheers,
today's sheriff

Original issue's description:
> Make the repeat button accessible by keyboard.
> As a known issue, duplicated ripple effect will be shown when clicking the
icon (instead of keyboard).
> 
> BUG=662829
> TEST=manual test by hitting [TAB] key several times
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
> 
> Committed: https://crrev.com/b5275a7dcc189413073ea961c1eb7d3768ef35b8
> Cr-Commit-Position: refs/heads/master@{#430897}

TBR=fukino@chromium.org,oka@chromium.org,yamaguchi@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=662829
==========

to

==========
Revert of Make the repeat button accessible by keyboard. (patchset #7 id:120001
of https://codereview.chromium.org/2482063002/ )

Reason for revert:
Broke
https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%...,
more info in http://crbug.com/662829#c3.

Cheers,
today's sheriff

Original issue's description:
> Make the repeat button accessible by keyboard.
> As a known issue, duplicated ripple effect will be shown when clicking the
icon (instead of keyboard).
>
> BUG=662829
> TEST=manual test by hitting [TAB] key several times
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
>
> Committed: https://crrev.com/b5275a7dcc189413073ea961c1eb7d3768ef35b8
> Cr-Commit-Position: refs/heads/master@{#430897}

TBR=fukino@chromium.org,oka@chromium.org,yamaguchi@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=662829

Committed: https://crrev.com/76e2c525c63fcb6b744b0e2bb73394f5b17da4c7
Cr-Commit-Position: refs/heads/master@{#430913}
==========

[commit-bot: I haz the power, 2016-11-09 12:06:40]

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

[oka, 2016-11-10 02:33:22]

Thank you Vaclav.
Novice question: how can we prevent this happens going forward? Can we
register the failed integration test to run on trybot?

On Wed, Nov 9, 2016 at 9:06 PM commit-bot@chromium.org via
codereview.chromium.org <reply@chromiumcodereview-hr.appspotmail.com> wrote:

> Patchset 1 (id:??) landed as
> https://crrev.com/76e2c525c63fcb6b744b0e2bb73394f5b17da4c7
> Cr-Commit-Position: refs/heads/master@{#430913}
>
> https://codereview.chromium.org/2483243004/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[vabr (Chromium), 2016-11-10 07:55:51]

On 2016/11/10 02:33:22, oka wrote:
> Thank you Vaclav.
> Novice question: how can we prevent this happens going forward? Can we
> register the failed integration test to run on trybot?
> 

Hello oka@,

Normally, these tests should be captured within the trybot set of tests.
Looking at your original CL (patch set 7), the most relevant trybot seems to be
linux_chromium_chromeos_rel_ng. This one, however, decided not to even compile
[1]. This might be a mistake, or on purpose (some tests are missing on trybots
to save computational resources). You can clarify this with Chrome Infra, if you
want to. Doing so might prevent future breakages for other people as well.

When you are testing your fix before relanding the original patch, try to run
the failing test locally, if you can. You just need a GNU/Linux machine for
that, set up a build for CrOS (target_os = "chromeos" in GN args) and run
./browser_tests --gtest_filter='OpenAudioFiles/FileManagerBrowserTest.Test/4' in
the corresponding out/... directory.

In general, running trybots is the best you can do, so in cases like this, it is
OK when the sheriff needs to revert in case more tests after landing discovered
some issues.

Thanks!
Vaclav

[1]
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...

[oka, 2016-11-10 12:30:33]

Thank you. I sent a post to chromium-discuss.
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-discuss/a3Uf1...

On Thu, Nov 10, 2016 at 4:55 PM <vabr@chromium.org> wrote:

On 2016/11/10 02:33:22, oka wrote:
> Thank you Vaclav.
> Novice question: how can we prevent this happens going forward? Can we
> register the failed integration test to run on trybot?
>

Hello oka@,

Normally, these tests should be captured within the trybot set of tests.
Looking at your original CL (patch set 7), the most relevant trybot seems
to be
linux_chromium_chromeos_rel_ng. This one, however, decided not to even
compile
[1]. This might be a mistake, or on purpose (some tests are missing on
trybots
to save computational resources). You can clarify this with Chrome Infra,
if you
want to. Doing so might prevent future breakages for other people as well.

When you are testing your fix before relanding the original patch, try to
run
the failing test locally, if you can. You just need a GNU/Linux machine for
that, set up a build for CrOS (target_os = "chromeos" in GN args) and run
./browser_tests
--gtest_filter='OpenAudioFiles/FileManagerBrowserTest.Test/4' in
the corresponding out/... directory.

In general, running trybots is the best you can do, so in cases like this,
it is
OK when the sheriff needs to revert in case more tests after landing
discovered
some issues.

Thanks!
Vaclav

[1]
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...

https://codereview.chromium.org/2483243004/

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.