In rebaseline.py, update SCM and expectations all at once after all commands.

In rebaseline.py, update SCM and expectations all at once after all commands.

This is meant to fix http://crbug.com/639410 by making it so that if there are files that are both added and deleted (e.g. downloaded and then de-duped), they won't ever be staged since all changes would be aggregated and de-duped before running git add/rm.

BUG=639410
Committed: https://crrev.com/34fdc9df872c6ce13e6c3c546084435321d4e882
Cr-Commit-Position: refs/heads/master@{#420461}

[qyearsley, 2016-09-16 00:13:30]

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

[commit-bot: I haz the power, 2016-09-16 00:14:06]

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

[qyearsley, 2016-09-16 00:26:55]

Description was changed from

==========
Update scm changes and expectation line changes all at once at the end of
_rebaseline.

BUG=639410
==========

to

==========
In rebaseline.py, update SCM and expectations all at once after all commands.

This is meant to fix http://crbug.com/639410 by making it so that if there are
files that are both added and deleted (e.g. downloaded and then de-duped), they
won't ever be staged since all changes would be aggregated and de-duped before
running git add/rm.

BUG=639410
==========

[qyearsley, 2016-09-16 00:26:55]

qyearsley@chromium.org changed reviewers:
+ dpranke@chromium.org, wkorman@chromium.org

[qyearsley, 2016-09-16 00:28:17]

Note: I haven't yet added a unit test for the ChangeSet class -- I could add
that in this CL or a separate CL.

[Dirk Pranke, 2016-09-16 00:35:47]

lgtm

[wkorman, 2016-09-16 00:37:53]

I would vote for adding test for ChangeSet to this CL.

Since, I think our existing unit test coverage for the permutations of what we
could get back from optimize_baselines is limited, and so if we can at least
gain confidence that ChangeSet has solidarity to it we can feel better about
things.

Also do we check the result code for the child processes we spin off for
optimize-baselines, and what do we do if one or more of them fail?

[commit-bot: I haz the power, 2016-09-16 01:35:36]

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

[commit-bot: I haz the power, 2016-09-16 01:35:37]

Dry run: This issue passed the CQ dry run.

[qyearsley, 2016-09-16 23:18:31]

On 2016/09/16 at 00:37:53, wkorman wrote:
> I would vote for adding test for ChangeSet to this CL.
> 
> Since, I think our existing unit test coverage for the permutations of what we
could get back from optimize_baselines is limited, and so if we can at least
gain confidence that ChangeSet has solidarity to it we can feel better about
things.
> 
> Also do we check the result code for the child processes we spin off for
optimize-baselines, and what do we do if one or more of them fail?

Makes sense, will do. I believe that for the child processes for
optimize-baselines, failures might be ignored, but not sure currently, will also
try to add test coverage for that.

[qyearsley, 2016-09-22 17:53:59]

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

[commit-bot: I haz the power, 2016-09-22 17:54:28]

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

[qyearsley, 2016-09-22 18:10:13]

On 2016/09/16 at 23:18:31, qyearsley (OOO til Sept 20) wrote:
> On 2016/09/16 at 00:37:53, wkorman wrote:
> > I would vote for adding test for ChangeSet to this CL.
> > 
> > Since, I think our existing unit test coverage for the permutations of what
we could get back from optimize_baselines is limited, and so if we can at least
gain confidence that ChangeSet has solidarity to it we can feel better about
things.
> > 
> > Also do we check the result code for the child processes we spin off for
optimize-baselines, and what do we do if one or more of them fail?
> 
> Makes sense, will do. I believe that for the child processes for
optimize-baselines, failures might be ignored, but not sure currently, will also
try to add test coverage for that.

Now added unit test for ChangeSet to this CL.

On inspection, I think that if sub-commands fail, then rebaselining will
generally fail silently and just not do something (e.g. not download baseline,
not de-dupe); filed bug http://crbug.com/649412 for this.

Does everything still look OK to commit?

[commit-bot: I haz the power, 2016-09-22 18:45:11]

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

[commit-bot: I haz the power, 2016-09-22 18:45:12]

Dry run: This issue passed the CQ dry run.

[qyearsley, 2016-09-22 21:17:35]

The CQ bit was checked by qyearsley@chromium.org

[qyearsley, 2016-09-22 21:17:35]

The patchset sent to the CQ was uploaded after l-g-t-m from dpranke@chromium.org
Link to the patchset: https://codereview.chromium.org/2341643005/#ps20001
(title: "Add unit test for ChangeSet")

[commit-bot: I haz the power, 2016-09-22 21:18:15]

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-09-22 21:24:45]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-22 21:28:32]

Description was changed from

==========
In rebaseline.py, update SCM and expectations all at once after all commands.

This is meant to fix http://crbug.com/639410 by making it so that if there are
files that are both added and deleted (e.g. downloaded and then de-duped), they
won't ever be staged since all changes would be aggregated and de-duped before
running git add/rm.

BUG=639410
==========

to

==========
In rebaseline.py, update SCM and expectations all at once after all commands.

This is meant to fix http://crbug.com/639410 by making it so that if there are
files that are both added and deleted (e.g. downloaded and then de-duped), they
won't ever be staged since all changes would be aggregated and de-duped before
running git add/rm.

BUG=639410

Committed: https://crrev.com/34fdc9df872c6ce13e6c3c546084435321d4e882
Cr-Commit-Position: refs/heads/master@{#420461}
==========

[commit-bot: I haz the power, 2016-09-22 21:28:33]

Patchset 2 (id:??) landed as
https://crrev.com/34fdc9df872c6ce13e6c3c546084435321d4e882
Cr-Commit-Position: refs/heads/master@{#420461}

[qyearsley, 2016-09-28 16:03:18]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/2380533003/ by qyearsley@chromium.org.

The reason for reverting is: Caused rebaseline-o-matic to fail:
http://crrev.com/650944

Could be re-landed with a fix..