[Culprit-finder] Refactor findit testcase and move it to libs.

[Culprit-finder] Refactor findit testcase and move it to libs.

TBR=stgao@chromium.org, lijeffrey@chromium.org
BUG=659449
Committed: https://chromium.googlesource.com/infra/infra/+/c04e4a7c1d39377b163da6219106b365ca2b3b25

[Sharu Jiang, 2016-11-22 03:12:27]

Patchset #1 (id:1) has been deleted

[Sharu Jiang, 2016-11-22 03:14:10]

katesonia@chromium.org changed reviewers:
+ chanli@chromium.org, lijeffrey@chromium.org, stgao@chromium.org,
wrengr@chromium.org

[Sharu Jiang, 2016-11-22 03:14:11]

PTAL :)

[Sharu Jiang, 2016-11-22 03:34:29]

Description was changed from

==========
[Culprit-finder] Refactor findit testcase in common, move it to libs.

BUG=
==========

to

==========
[Culprit-finder] Refactor findit testcase in common, move it to libs.

BUG=
==========

[Sharu Jiang, 2016-11-22 03:34:55]

Description was changed from

==========
[Culprit-finder] Refactor findit testcase in common, move it to libs.

BUG=
==========

to

==========
[Culprit-finder] Refactor findit testcase and move it to libs.

BUG=659449
==========

[wrengr, 2016-11-22 19:24:54]

lgtm

[chanli, 2016-11-23 06:02:45]

On 2016/11/22 19:24:54, wrengr wrote:
> lgtm

Generally LGTM. But since you have added MockHttpClient, I think there are more
tests can use the testcase.

[Sharu Jiang, 2016-11-23 19:15:30]

Description was changed from

==========
[Culprit-finder] Refactor findit testcase and move it to libs.

BUG=659449
==========

to

==========
[Culprit-finder] Refactor findit testcase and move it to libs.

TBR=stgao@chromium.org, lijeffrey@chromium.org
BUG=659449
==========

[Sharu Jiang, 2016-11-23 19:15:40]

The CQ bit was checked by katesonia@chromium.org

[Sharu Jiang, 2016-11-23 19:15:40]

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

[commit-bot: I haz the power, 2016-11-23 19:15:42]

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-23 19:31:02]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1479928540138170,
"parent_rev": "57bb0438f26ea165de1196a5430438369fd98d61", "commit_rev":
"c04e4a7c1d39377b163da6219106b365ca2b3b25"}

[commit-bot: I haz the power, 2016-11-23 19:31:05]

Description was changed from

==========
[Culprit-finder] Refactor findit testcase and move it to libs.

TBR=stgao@chromium.org, lijeffrey@chromium.org
BUG=659449
==========

to

==========
[Culprit-finder] Refactor findit testcase and move it to libs.

TBR=stgao@chromium.org, lijeffrey@chromium.org
BUG=659449

Committed:
https://chromium.googlesource.com/infra/infra/+/c04e4a7c1d39377b163da6219106b...
==========

[commit-bot: I haz the power, 2016-11-23 19:31:06]

Committed patchset #2 (id:40001) as
https://chromium.googlesource.com/infra/infra/+/c04e4a7c1d39377b163da6219106b...

[stgao, 2016-11-23 20:24:53]

https://codereview.chromium.org/2521773004/diff/40001/appengine/findit/libs/t...
File appengine/findit/libs/testcase.py (right):

https://codereview.chromium.org/2521773004/diff/40001/appengine/findit/libs/t...
appengine/findit/libs/testcase.py:7: from testing_utils import testing
But this depends on App Engine local test setup, while libs is for non-appengine
code.

Ideally this should goes to gae_libs/ instead.

[Sharu Jiang, 2016-11-23 20:55:00]

https://codereview.chromium.org/2521773004/diff/40001/appengine/findit/libs/t...
File appengine/findit/libs/testcase.py (right):

https://codereview.chromium.org/2521773004/diff/40001/appengine/findit/libs/t...
appengine/findit/libs/testcase.py:7: from testing_utils import testing
On 2016/11/23 20:24:53, stgao (slow on Monday) wrote:
> But this depends on App Engine local test setup, while libs is for
non-appengine
> code.
> 
> Ideally this should goes to gae_libs/ instead.

Right, that makes sense, move it in another cl
https://codereview.chromium.org/2525003005/.

[Sharu Jiang, 2016-11-24 02:07:05]

On 2016/11/23 06:02:45, chanli wrote:
> On 2016/11/22 19:24:54, wrengr wrote:
> > lgtm
> 
> Generally LGTM. But since you have added MockHttpClient, I think there are
more
> tests can use the testcase.

Right, I found several more tests, we could refactor them in following cls.
Filed a bug to track this
https://bugs.chromium.org/p/chromium/issues/detail?id=668339