move private test for sect_with_horizontal into unittests

move private test for sect_with_horizontal into unittests

BUG=638575
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2250353004

Committed: https://skia.googlesource.com/skia/+/ff863bc550a8738c9c2667be225e653636617a90

[reed1, 2016-08-17 13:08:32]

Description was changed from

==========
don't call inner test 'cause the static global is tsan racey, but leave it for
documentation

BUG=638575
==========

to

==========
don't call inner test 'cause the static global is tsan racey, but leave it for
documentation

BUG=638575
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2250353004
==========

[reed1, 2016-08-17 13:08:49]

reed@google.com changed reviewers:
+ mtklein@google.com

[reed1, 2016-08-17 13:08:54]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-17 13:09:02]

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

[mtklein, 2016-08-17 13:12:53]

https://codereview.chromium.org/2250353004/diff/1/src/core/SkLineClipper.cpp
File src/core/SkLineClipper.cpp (left):

https://codereview.chromium.org/2250353004/diff/1/src/core/SkLineClipper.cpp#...
src/core/SkLineClipper.cpp:180: static bool gOnce;
If you want to keep this and make it thread safe, you should be able to just
write:

    static SkOnce once;
    once(sect_with_horizontal_test_for_pin_results);

[commit-bot: I haz the power, 2016-08-17 13:31:52]

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

[commit-bot: I haz the power, 2016-08-17 13:31:53]

Dry run: This issue passed the CQ dry run.

[reed1, 2016-08-17 13:57:43]

Description was changed from

==========
don't call inner test 'cause the static global is tsan racey, but leave it for
documentation

BUG=638575
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2250353004
==========

to

==========
move private test for sect_with_horizontal into unittests

BUG=638575
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2250353004
==========

[reed1, 2016-08-17 13:58:09]

moved the test to make it "real" -- and confirmed that it exercises what it says
it does.

[reed1, 2016-08-17 13:58:13]

The CQ bit was checked by reed@google.com

[commit-bot: I haz the power, 2016-08-17 13:58:23]

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

[mtklein, 2016-08-17 14:06:23]

https://codereview.chromium.org/2250353004/diff/20001/tests/PathTest.cpp
File tests/PathTest.cpp (right):

https://codereview.chromium.org/2250353004/diff/20001/tests/PathTest.cpp#newc...
tests/PathTest.cpp:107: }
We've dropped any sort of assert here... is this just tested by existing
SkASSERTs?

https://codereview.chromium.org/2250353004/diff/20001/tests/PathTest.cpp#newc...
tests/PathTest.cpp:121: make_path_crbug638575(&path);
It doesn't look like this has anything to do with 364224.  Why don't we make it
independent?

static void test_crbug658575() {
    SkPath path;
    path.moveTo(...);
    path.lineTo(...);
    path.lineTo(...);
    // By not asserting in this function, we have tested that
sect_with_horizontal in SkLineClipper.cpp needs to pin after computing the
intersection.
}

[reed1, 2016-08-17 14:15:02]

https://codereview.chromium.org/2250353004/diff/20001/tests/PathTest.cpp
File tests/PathTest.cpp (right):

https://codereview.chromium.org/2250353004/diff/20001/tests/PathTest.cpp#newc...
tests/PathTest.cpp:107: }
On 2016/08/17 14:06:23, mtklein wrote:
> We've dropped any sort of assert here... is this just tested by existing
> SkASSERTs?

The line-clipper itself is loaded with the appropriate asserts already, just
needed to send in the right numbers to trigger them. This is the premiss for all
of the path-tests in this area of this file.

I'll add a comment to that effect.

https://codereview.chromium.org/2250353004/diff/20001/tests/PathTest.cpp#newc...
tests/PathTest.cpp:121: make_path_crbug638575(&path);
On 2016/08/17 14:06:23, mtklein wrote:
> It doesn't look like this has anything to do with 364224.  Why don't we make
it
> independent?
> 
> static void test_crbug658575() {
>     SkPath path;
>     path.moveTo(...);
>     path.lineTo(...);
>     path.lineTo(...);
>     // By not asserting in this function, we have tested that
> sect_with_horizontal in SkLineClipper.cpp needs to pin after computing the
> intersection.
> }

True enough. That bug pointed this out, but really the CL now is just a
refactor. I'll rename the test appropriately.

[reed1, 2016-08-17 14:15:06]

The CQ bit was checked by reed@google.com

[commit-bot: I haz the power, 2016-08-17 14:15:15]

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

[mtklein, 2016-08-17 14:19:34]

lgtm

[commit-bot: I haz the power, 2016-08-17 14:38:00]

Description was changed from

==========
move private test for sect_with_horizontal into unittests

BUG=638575
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2250353004
==========

to

==========
move private test for sect_with_horizontal into unittests

BUG=638575
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2250353004

Committed:
https://skia.googlesource.com/skia/+/ff863bc550a8738c9c2667be225e653636617a90
==========

[commit-bot: I haz the power, 2016-08-17 14:38:00]

Committed patchset #3 (id:40001) as
https://skia.googlesource.com/skia/+/ff863bc550a8738c9c2667be225e653636617a90