Time for some cleanup in LargeIconServiceTest. Ideas:
pkotwicz@:
- Make a local test version of
GetLargeIconOrFallbackStyleFromGoogleServerSkippingLocalCache() which takes a
struct. That way you only need to specify non default values
- Build |kExpectedServerUrlWithoutCheck| from a "default expected URL".
e.g.
const GURL kExpectedServerUrlWithoutCheck =
ReplaceQueryParameter(kDefaultExpectedServerUrl, "check_seen=true", "");
Of course, ReplaceQueryParameter() could be more complicated
mastiz@:
I feel that the most severe
duplication here is not about the test fixture (e.g. irrelevant
|desired_size_in_pixel|) but the redundant test expectations
(|kExpectedServerUrl|). Except perhaps minor and trivial improvements like
CreateTestSkBitmap() taking default values or returning gfx::Image directly
(CreateTestImage()).
I think what we're trying to do here is to verify specifically one URL
parameter, which is "check_seen". Hence, I'd suggest either using
testing::HasSubstr() or preferably introducing a custom matcher,
QueryParameterEq("check_seen", _). This could be combined with the adoption of
ON_CALL for StartOrQueueNetworkRequest(_, _, _, _).
Comment 1 by jkrcal@chromium.org
, Feb 26 2018