Clean up cc test harnesses with lots of methods/helpers |
||
Issue descriptionSo we have this LayerTreeHostCommonTestBase class with methods to set 7 diff properties on a layer. We can just set things on the layer directly instead of having to pass a bunch of things that have defaults, and make it hard to tell from the caller which thing is what. Also it has many ways to call CalcDrawProps and I feel we could make that simpler. Also also this class is actually just for the layer_tree_host_common_unittests.cc file. So it should live in that file not in cc/test/ or other tests will use it and we have too many *LayerTreeHost* things in cc/test/ that it makes it confusing for people trying to find things to use in there IMHO. I'm going to poke at LayerTreeHostCommonTestBase. vmpstr@ says there's some similar things in PictureLayerTiling tests.
,
Jul 26 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f78fb2721e1983a054d169c14557cd814dcea915 commit f78fb2721e1983a054d169c14557cd814dcea915 Author: danakj <danakj@chromium.org> Date: Tue Jul 26 19:06:15 2016 cc: Remove SetLayerPropertiesForTesting from LayerTreeHostCommonTestBase Replaces all remaining uses with direct set function calls onto the Layer/LayerImpl. R=ajuma@chromium.org, jaydasika@chromium.org BUG=630754 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel Review-Url: https://codereview.chromium.org/2179263002 Cr-Commit-Position: refs/heads/master@{#407873} [modify] https://crrev.com/f78fb2721e1983a054d169c14557cd814dcea915/cc/test/layer_tree_host_common_test.cc [modify] https://crrev.com/f78fb2721e1983a054d169c14557cd814dcea915/cc/test/layer_tree_host_common_test.h [modify] https://crrev.com/f78fb2721e1983a054d169c14557cd814dcea915/cc/trees/layer_tree_host_common_unittest.cc [modify] https://crrev.com/f78fb2721e1983a054d169c14557cd814dcea915/cc/trees/layer_tree_impl_unittest.cc
,
Jul 26 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/59931941b4a767b78269fa0d1d5fccbba76158c3 commit 59931941b4a767b78269fa0d1d5fccbba76158c3 Author: danakj <danakj@chromium.org> Date: Tue Jul 26 22:11:29 2016 cc: Move LayerTreeHostCommonTest harness into the LTHCommon tests file This moves the tests baseclass into the file where the tests live, so we don't have test base classes living in cc/test/. This is done because base classes separate from the tests themselves is confusing to people trying to write new tests, there's just so many LayerTreeHost* type things in cc/test/ that it's unclear what to use. This resulted in the duplication of (part of) one function into LayerTreeImplTest to ExecuteCalculateDrawProperties(). Also delete some an unused class, MockContentLayerClient, and an unused macro. And remove the EXPECT_IDEAL_CONTENT_SCALE macro, just using EXPECT_FLOAT_EQ instead as the macro has become a very simple proxy. I intend to make these suites stop inheriting from the LayerTestCommon::LayerImplTest class also, as it is meant to be used as composition not inheritence. R=ajuma@chromium.org, jaydasika@chromium.org BUG=630754 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel Review-Url: https://codereview.chromium.org/2185463002 Cr-Commit-Position: refs/heads/master@{#407929} [modify] https://crrev.com/59931941b4a767b78269fa0d1d5fccbba76158c3/cc/BUILD.gn [modify] https://crrev.com/59931941b4a767b78269fa0d1d5fccbba76158c3/cc/cc_tests.gyp [delete] https://crrev.com/786c89d33f00bc8097a40180f0f3effbe8ec42b6/cc/test/layer_tree_host_common_test.cc [delete] https://crrev.com/786c89d33f00bc8097a40180f0f3effbe8ec42b6/cc/test/layer_tree_host_common_test.h [modify] https://crrev.com/59931941b4a767b78269fa0d1d5fccbba76158c3/cc/trees/layer_tree_host_common_unittest.cc [modify] https://crrev.com/59931941b4a767b78269fa0d1d5fccbba76158c3/cc/trees/layer_tree_impl_unittest.cc
,
Jul 26 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/76ddc8770c3b80af44f97ddaa32d14098beb0ff8 commit 76ddc8770c3b80af44f97ddaa32d14098beb0ff8 Author: danakj <danakj@chromium.org> Date: Tue Jul 26 23:21:43 2016 cc: Make LayerTreeImpl tests stop subclassing LayerImplTest. This class is meant for composition not inheritance. R=ajuma, jaydasika@chromium.org BUG=630754 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel Review-Url: https://codereview.chromium.org/2181763005 Cr-Commit-Position: refs/heads/master@{#407961} [modify] https://crrev.com/76ddc8770c3b80af44f97ddaa32d14098beb0ff8/cc/test/layer_test_common.cc [modify] https://crrev.com/76ddc8770c3b80af44f97ddaa32d14098beb0ff8/cc/trees/layer_tree_impl_unittest.cc
,
Jul 30 2016
Hm, we also have TestLayerTreeHostBase, which at first glance looks like an impl-only version of LayerTreeTest, but doesn't mention LTHI in its name. Also it inherits from testing::Test which it'd be nice if individual tests did instead. It overlaps quite a bit with LayerImplTest from the looks of it.
,
Aug 5 2016
|
||
►
Sign in to add a comment |
||
Comment 1 by bugdroid1@chromium.org
, Jul 25 2016