Isolate remaining tests under ui/

Isolate remaining tests under ui/

BUG=98637
R=maruel@chromium.org,jam@chromium.org

Committed: https://crrev.com/4d293d79bd67b683942fd727c46f2b3173d7f03e
Cr-Commit-Position: refs/heads/master@{#307954}

[jochen (gone - plz use gerrit), 2014-12-08 07:50:43]

jochen@chromium.org changed reviewers:
+ jam@chromium.org, maruel@chromium.org

[jochen (gone - plz use gerrit), 2014-12-08 08:02:11]

ptal

[jochen (gone - plz use gerrit), 2014-12-08 11:59:12]

jochen@chromium.org changed reviewers:
+ phajdan.jr@chromium.org

[Paweł Hajdan Jr., 2014-12-08 14:09:50]

Looks fine but there are errors on trybots, so I'd expect there'd be some
further changes to this CL.

[M-A Ruel, 2014-12-08 14:53:48]

On 2014/12/08 14:09:50, Paweł Hajdan Jr. wrote:
> Looks fine but there are errors on trybots, so I'd expect there'd be some
> further changes to this CL.

Generally looks good. The failures looks real though, for example;
MappingError: Input file
/Volumes/data/b/build/slave/mac/build/src/out/Release/ui/en-US.pak doesn't exist

One tricky thing is that you may need to add missing gyp target dependencies. It
happened in the past. You can confirm locally by doing a clean build with

GYP_DEFINES=test_isolation_mode=check ninja -C out/Release my_unittest_run

Thanks for doing this, it's really appreciated.

[M-A Ruel, 2014-12-08 14:56:16]

https://codereview.chromium.org/724603004/diff/240001/ui/accessibility/access...
File ui/accessibility/accessibility_unittests.isolate (right):

https://codereview.chromium.org/724603004/diff/240001/ui/accessibility/access...
ui/accessibility/accessibility_unittests.isolate:6: ['OS=="android" or
OS=="linux" or OS=="mac" or OS=="win"', {
I don't think this test will ever be executed on android, so having this
condition could be misleading to someone who reads this file. I'd recommend to
remove android on each new file you added.

https://codereview.chromium.org/724603004/diff/240001/ui/gfx/gfx_unittests.is...
File ui/gfx/gfx_unittests.isolate (right):

https://codereview.chromium.org/724603004/diff/240001/ui/gfx/gfx_unittests.is...
ui/gfx/gfx_unittests.isolate:14: '<(PRODUCT_DIR)/ui_test.pak',
Maybe worth a ui.isolate to contain this? I saw it in a few files. Then
ui.isolate can includes base.isolate. On the other hand I see that
event_unittests doesn't need it so maybe it's not a that great idea after all,
so ignore this comment.

[jam, 2014-12-08 18:10:59]

lgtm (i trust you'll only land this when it's green :) )

[jochen (gone - plz use gerrit), 2014-12-11 18:36:37]

as green as it gets.

the linux_chromium_chromeos_rel fails without my patch, the asan bots fails
browser tests (i didn't touch code)

any last comments? I'll put this into the CQ tomorrow (my time)

On Mon Dec 08 2014 at 7:11:00 PM <jam@chromium.org> wrote:

> lgtm (i trust you'll only land this when it's green :) )
>
> https://codereview.chromium.org/724603004/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[M-A Ruel, 2014-12-11 18:37:55]

On 2014/12/11 18:36:37, jochen (slow) wrote:
> as green as it gets.
> 
> the linux_chromium_chromeos_rel fails without my patch, the asan bots fails
> browser tests (i didn't touch code)
> 
> any last comments? I'll put this into the CQ tomorrow (my time)

lgtm, I'd prefer to CQ right away.

[M-A Ruel, 2014-12-11 18:41:44]

The CQ bit was checked by maruel@chromium.org

[commit-bot: I haz the power, 2014-12-11 18:43:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/724603004/400001

[commit-bot: I haz the power, 2014-12-11 19:54:58]

Committed patchset #21 (id:400001)

[commit-bot: I haz the power, 2014-12-11 19:55:50]

Patchset 21 (id:??) landed as
https://crrev.com/4d293d79bd67b683942fd727c46f2b3173d7f03e
Cr-Commit-Position: refs/heads/master@{#307954}

[M-A Ruel, 2014-12-12 14:01:38]

A revert of this CL (patchset #21 id:400001) has been created in
https://codereview.chromium.org/802673002/ by maruel@chromium.org.

The reason for reverting is: Speculatively reverting due to chromium.win not
triggered testers at all.

http://build.chromium.org/p/chromium.win/builders/Win%20Builder/builds/5439/
succeeded in triggering testers
http://build.chromium.org/p/chromium.win/builders/Win%20Builder/builds/5440/
didn't.

There was no build.git commit nor recipe change.

BUG=441690.