Cleanup expected.txt files for layout tests

Cleanup expected.txt files for layout tests

Rework the layout tests for origin trials so the expected.txt files are
no longer needed. This was addressing the harness error about duplicate
test names. Due to tests being run in multiple workers, the structure
was reworked to make it easier to create unique test names.

Also, remove the expectation for a flaky test, since the expected.txt
files were forcing the flaky results (see crbug.com/705953).

This is a precursor so later changes to the layout tests do not require
changing multiple expected.txt files.

BUG=695123, 705953
Review-Url: https://codereview.chromium.org/2808543002
Cr-Commit-Position: refs/heads/master@{#463525}
Committed: https://chromium.googlesource.com/chromium/src/+/61358d56517577686b5003b89146cc2071f79ed1

[chasej, 2017-04-07 19:25:52]

The CQ bit was checked by chasej@chromium.org to run a CQ dry run

[chasej, 2017-04-07 19:26:33]

chasej@chromium.org changed reviewers:
+ iclelland@chromium.org

[chasej, 2017-04-07 19:26:33]

iclelland, please take a look.

[commit-bot: I haz the power, 2017-04-07 19:26:56]

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

[iclelland, 2017-04-07 20:09:51]

On 2017/04/07 19:26:33, chasej wrote:
> iclelland, please take a look.

This looks really good -- thanks for cleaning that up! Two quick questions:
 - the CL would be about 10 files smaller if it weren't for the
'origin_trials.js' -> 'origintrials.js' rename -- is something motivating that
change?
 - The methods in origintrials-worker.js take a 'global' as a parameter, but the
only argument ever passed to them is the actual global object, which they could
have used directly. Are there plans to ever pass a different object in to those
methods? Could they just use the global 'self'?

[commit-bot: I haz the power, 2017-04-07 20:30:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-07 20:30:54]

Dry run: This issue passed the CQ dry run.

[chasej, 2017-04-07 20:41:01]

On 2017/04/07 20:09:51, iclelland wrote:
> On 2017/04/07 19:26:33, chasej wrote:
> > iclelland, please take a look.
> 
> This looks really good -- thanks for cleaning that up! Two quick questions:
>  - the CL would be about 10 files smaller if it weren't for the
> 'origin_trials.js' -> 'origintrials.js' rename -- is something motivating that
> change?
Yes, it was to make the file naming use consistent separators. For whatever
reason, 'origin_trials.js' was the only file using an underscore as a separator.
I started to add 'origin_trials_worker.js', but that looked odd next to
'enabled-worker.js', and so on. I debated, but given this is a separate cleanup
CL
anyways, seems like the right time.

>  - The methods in origintrials-worker.js take a 'global' as a parameter, but
the
> only argument ever passed to them is the actual global object, which they
could
> have used directly. Are there plans to ever pass a different object in to
those
> methods? Could they just use the global 'self'?
I don't have specific plans to pass a different object. Because this file is
kind
of nested (e.g. two importScripts() from the actual load site), I preferred to 
avoid the direct use of 'self'. But, I forgot that the test calls were already
using 'self'. I've removed the parameter to simplify.

[chasej, 2017-04-07 20:46:53]

Description was changed from

==========
Cleanup expected.txt files for layout tests

Rework the layout tests for origin trials so the expected.txt files are
no longer needed. This was addressing the harness error about duplicate
test names. Due to tests being run in multiple workers, the structure
was reworked to make it easier to create unique test names.

This is a precursor so later changes to the layout tests do not require
changing multiple expected.txt files.

BUG=695123
==========

to

==========
Cleanup expected.txt files for layout tests

Rework the layout tests for origin trials so the expected.txt files are
no longer needed. This was addressing the harness error about duplicate
test names. Due to tests being run in multiple workers, the structure
was reworked to make it easier to create unique test names.

This is a precursor so later changes to the layout tests do not require
changing multiple expected.txt files.

BUG=695123, 705953
==========

[chasej, 2017-04-07 20:50:38]

The CQ bit was checked by chasej@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-07 20:51:04]

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

[chasej, 2017-04-07 20:53:06]

Description was changed from

==========
Cleanup expected.txt files for layout tests

Rework the layout tests for origin trials so the expected.txt files are
no longer needed. This was addressing the harness error about duplicate
test names. Due to tests being run in multiple workers, the structure
was reworked to make it easier to create unique test names.

This is a precursor so later changes to the layout tests do not require
changing multiple expected.txt files.

BUG=695123, 705953
==========

to

==========
Cleanup expected.txt files for layout tests

Rework the layout tests for origin trials so the expected.txt files are
no longer needed. This was addressing the harness error about duplicate
test names. Due to tests being run in multiple workers, the structure
was reworked to make it easier to create unique test names.

Also, remove the expectation for a flaky test, since the expected.txt
files were forcing the flaky results (see crbug.com/705953).

This is a precursor so later changes to the layout tests do not require
changing multiple expected.txt files.

BUG=695123, 705953
==========

[iclelland, 2017-04-07 20:53:20]

On 2017/04/07 20:41:01, chasej wrote:
> On 2017/04/07 20:09:51, iclelland wrote:
> > On 2017/04/07 19:26:33, chasej wrote:
> > > iclelland, please take a look.
> > 
> > This looks really good -- thanks for cleaning that up! Two quick questions:
> >  - the CL would be about 10 files smaller if it weren't for the
> > 'origin_trials.js' -> 'origintrials.js' rename -- is something motivating
that
> > change?
> Yes, it was to make the file naming use consistent separators. For whatever
> reason, 'origin_trials.js' was the only file using an underscore as a
separator.
> I started to add 'origin_trials_worker.js', but that looked odd next to
> 'enabled-worker.js', and so on. I debated, but given this is a separate
cleanup
> CL
> anyways, seems like the right time.
> 
> >  - The methods in origintrials-worker.js take a 'global' as a parameter, but
> the
> > only argument ever passed to them is the actual global object, which they
> could
> > have used directly. Are there plans to ever pass a different object in to
> those
> > methods? Could they just use the global 'self'?
> I don't have specific plans to pass a different object. Because this file is
> kind
> of nested (e.g. two importScripts() from the actual load site), I preferred to

> avoid the direct use of 'self'. But, I forgot that the test calls were already
> using 'self'. I've removed the parameter to simplify.

I suppose I would have been okay with it either way; I just wanted to know if
there was a reason for it (maybe testing the test methods some day? :) )

Thanks
lgtm

[commit-bot: I haz the power, 2017-04-07 22:38:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-07 22:38:23]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[chasej, 2017-04-10 18:14:08]

The CQ bit was checked by chasej@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-10 18:14:45]

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

[chasej, 2017-04-10 18:39:46]

The CQ bit was checked by chasej@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-10 18:40:30]

Dry run: 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, 2017-04-10 20:29:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-10 20:29:46]

Dry run: This issue passed the CQ dry run.

[chasej, 2017-04-10 20:48:38]

The CQ bit was checked by chasej@chromium.org

[chasej, 2017-04-10 20:48:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
iclelland@chromium.org
Link to the patchset: https://codereview.chromium.org/2808543002/#ps80001
(title: "Remove platform-specific expected.txt file")

[commit-bot: I haz the power, 2017-04-10 20:49:31]

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, 2017-04-10 20:57:14]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1491857318575300,
"parent_rev": "da1d85c561f4c7b666dfc1ae5acb3310d9b78a20", "commit_rev":
"dedaa0449ab884605eade44b14a3cf3e24b17548"}

[commit-bot: I haz the power, 2017-04-10 21:08:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-10 21:08:08]

Prior attempt to commit was detected, but we were not able to check whether the
issue was successfully committed. Please check Git history manually and re-check
CQ or close this issue as needed.

[chasej, 2017-04-11 03:56:04]

The CQ bit was checked by chasej@chromium.org

[commit-bot: I haz the power, 2017-04-11 03:56:39]

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, 2017-04-11 04:03:58]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1491882964763660,
"parent_rev": "fb21662ee5c04e2bfa9de9e4b9a86e5f101c524e", "commit_rev":
"61358d56517577686b5003b89146cc2071f79ed1"}

[commit-bot: I haz the power, 2017-04-11 04:05:12]

Description was changed from

==========
Cleanup expected.txt files for layout tests

Rework the layout tests for origin trials so the expected.txt files are
no longer needed. This was addressing the harness error about duplicate
test names. Due to tests being run in multiple workers, the structure
was reworked to make it easier to create unique test names.

Also, remove the expectation for a flaky test, since the expected.txt
files were forcing the flaky results (see crbug.com/705953).

This is a precursor so later changes to the layout tests do not require
changing multiple expected.txt files.

BUG=695123, 705953
==========

to

==========
Cleanup expected.txt files for layout tests

Rework the layout tests for origin trials so the expected.txt files are
no longer needed. This was addressing the harness error about duplicate
test names. Due to tests being run in multiple workers, the structure
was reworked to make it easier to create unique test names.

Also, remove the expectation for a flaky test, since the expected.txt
files were forcing the flaky results (see crbug.com/705953).

This is a precursor so later changes to the layout tests do not require
changing multiple expected.txt files.

BUG=695123, 705953

Review-Url: https://codereview.chromium.org/2808543002
Cr-Commit-Position: refs/heads/master@{#463525}
Committed:
https://chromium.googlesource.com/chromium/src/+/61358d56517577686b5003b89146...
==========

[commit-bot: I haz the power, 2017-04-11 04:05:14]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/61358d56517577686b5003b89146...