Fix a bug where video animations were being merged into other animations

Fix a bug where video animations were being merged into other animations

This was because the clause to compute whether a proto expectation
was a video animation:

var isVideo = pe.containsTypeNames([INITIATOR_TYPE.VIDEO]);

would always return false because PE was the initiator type name, not
the type of any events in the associated events.

BUG=catapult:#706004

Review-Url: https://codereview.chromium.org/2778993004
Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/d3a91071e560106b98d072184a54276ae13713cc

[charliea (OOO until 10-5), 2017-03-28 19:44:27]

charliea@chromium.org changed reviewers:
+ benjhayden@chromium.org

[charliea (OOO until 10-5), 2017-03-28 19:44:28]

https://codereview.chromium.org/2778993004/diff/1/tracing/tracing/importer/fi...
File tracing/tracing/importer/find_input_expectations.html (left):

https://codereview.chromium.org/2778993004/diff/1/tracing/tracing/importer/fi...
tracing/tracing/importer/find_input_expectations.html:1283: INITIATOR_TYPE,
This was being exported, but lives in proto_expectations.html. It seemed
innocuous enough to remove in this CL.

https://codereview.chromium.org/2778993004/diff/1/tracing/tracing/importer/fi...
File tracing/tracing/importer/find_input_expectations.html (right):

https://codereview.chromium.org/2778993004/diff/1/tracing/tracing/importer/fi...
tracing/tracing/importer/find_input_expectations.html:1037: var newPEs = [];
I tried changing all of these vars to consts, which seems like it should have
worked. For some reason, it caused the unit tests to start failing, and I
couldn't figure out why :-/ It doesn't seem like we rebind anything?

[benjhayden, 2017-03-28 19:54:20]

1 nit, 1 question, lgtm, thanks!

https://codereview.chromium.org/2778993004/diff/1/tracing/tracing/importer/fi...
File tracing/tracing/importer/find_input_expectations.html (left):

https://codereview.chromium.org/2778993004/diff/1/tracing/tracing/importer/fi...
tracing/tracing/importer/find_input_expectations.html:1283: INITIATOR_TYPE,
On 2017/03/28 at 19:44:28, charliea wrote:
> This was being exported, but lives in proto_expectations.html. It seemed
innocuous enough to remove in this CL.

Sure, this can happen in this CL.
Can you grep for tr.importer.INITIATOR_TYPE to make sure?

https://codereview.chromium.org/2778993004/diff/1/tracing/tracing/importer/fi...
File tracing/tracing/importer/find_input_expectations.html (right):

https://codereview.chromium.org/2778993004/diff/1/tracing/tracing/importer/fi...
tracing/tracing/importer/find_input_expectations.html:1037: var newPEs = [];
On 2017/03/28 at 19:44:28, charliea wrote:
> I tried changing all of these vars to consts, which seems like it should have
worked. For some reason, it caused the unit tests to start failing, and I
couldn't figure out why :-/ It doesn't seem like we rebind anything?

Sorry, no idea.
Maybe not for this CL, but somebody could try replacing one var at a time and
rerunning the tests?

https://codereview.chromium.org/2778993004/diff/1/tracing/tracing/importer/us...
File tracing/tracing/importer/user_model_builder_test.html (right):

https://codereview.chromium.org/2778993004/diff/1/tracing/tracing/importer/us...
tracing/tracing/importer/user_model_builder_test.html:104:
verifier.customizeModelCallback = (model) => {
I thought we didn't put parens around single arguments?
Callback = model => {

[charliea (OOO until 10-5), 2017-03-29 14:16:59]

https://codereview.chromium.org/2778993004/diff/1/tracing/tracing/importer/fi...
File tracing/tracing/importer/find_input_expectations.html (left):

https://codereview.chromium.org/2778993004/diff/1/tracing/tracing/importer/fi...
tracing/tracing/importer/find_input_expectations.html:1283: INITIATOR_TYPE,
On 2017/03/28 19:54:20, benjhayden wrote:
> On 2017/03/28 at 19:44:28, charliea wrote:
> > This was being exported, but lives in proto_expectations.html. It seemed
> innocuous enough to remove in this CL.
> 
> Sure, this can happen in this CL.
> Can you grep for tr.importer.INITIATOR_TYPE to make sure?

Good call. It looks like this is indeed used elsewhere, so I'll handle it in a
follow-up CL.

https://codereview.chromium.org/2778993004/diff/1/tracing/tracing/importer/fi...
File tracing/tracing/importer/find_input_expectations.html (right):

https://codereview.chromium.org/2778993004/diff/1/tracing/tracing/importer/fi...
tracing/tracing/importer/find_input_expectations.html:1037: var newPEs = [];
On 2017/03/28 19:54:20, benjhayden wrote:
> On 2017/03/28 at 19:44:28, charliea wrote:
> > I tried changing all of these vars to consts, which seems like it should
have
> worked. For some reason, it caused the unit tests to start failing, and I
> couldn't figure out why :-/ It doesn't seem like we rebind anything?
> 
> Sorry, no idea.
> Maybe not for this CL, but somebody could try replacing one var at a time and
> rerunning the tests?

Acknowledged.

https://codereview.chromium.org/2778993004/diff/1/tracing/tracing/importer/us...
File tracing/tracing/importer/user_model_builder_test.html (right):

https://codereview.chromium.org/2778993004/diff/1/tracing/tracing/importer/us...
tracing/tracing/importer/user_model_builder_test.html:104:
verifier.customizeModelCallback = (model) => {
On 2017/03/28 19:54:20, benjhayden wrote:
> I thought we didn't put parens around single arguments?
> Callback = model => {

Fixed.

[charliea (OOO until 10-5), 2017-03-29 14:18:29]

The CQ bit was checked by charliea@chromium.org

[charliea (OOO until 10-5), 2017-03-29 14:18:29]

The patchset sent to the CQ was uploaded after l-g-t-m from
benjhayden@chromium.org
Link to the patchset: https://codereview.chromium.org/2778993004/#ps20001
(title: "Checkpoint.")

[commit-bot: I haz the power, 2017-03-29 14:18:37]

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-03-29 15:03:01]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1490797109609500,
"parent_rev": "83f447ccc94dc3a0998f62569933f909ec7b069f", "commit_rev":
"d3a91071e560106b98d072184a54276ae13713cc"}

[commit-bot: I haz the power, 2017-03-29 15:03:05]

Description was changed from

==========
Fix a bug where video animations were being merged into other animations

This was because the clause to compute whether a proto expectation
was a video animation:

var isVideo = pe.containsTypeNames([INITIATOR_TYPE.VIDEO]);

would always return false because PE was the initiator type name, not
the type of any events in the associated events.

BUG=catapult:#706004
==========

to

==========
Fix a bug where video animations were being merged into other animations

This was because the clause to compute whether a proto expectation
was a video animation:

var isVideo = pe.containsTypeNames([INITIATOR_TYPE.VIDEO]);

would always return false because PE was the initiator type name, not
the type of any events in the associated events.

BUG=catapult:#706004

Review-Url: https://codereview.chromium.org/2778993004
Committed:
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...
==========

[commit-bot: I haz the power, 2017-03-29 15:03:06]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...