Commit b8c54296 authored by Brian Sheedy's avatar Brian Sheedy Committed by Commit Bot

Support multiple corpora for gtest pixel tests

Adds support for specifying a non-default corpus when initializing a
SkiaGoldPixelDiff or BrowserSkiaGoldPixelDiff instance. This allows
users to separate their images, either to reduce clutter when looking
through results or to prevent internal results from being mirrored to
the public Gold instance.

This particular implementation is meant to be a temporary solution so
that users can be unblocked. A more robust implementation that will
catch public corpora being used in non-public settings, etc. will be
implemented later.

Bug: 1108465
Change-Id: I8f868bfb32cdf72cf9feccf64c7291f758c3ae5f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2314976Reviewed-by: default avatarScott Violet <sky@chromium.org>
Reviewed-by: default avatarSven Zheng <svenzheng@chromium.org>
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791277}
parent 8ebd7836
...@@ -31,8 +31,9 @@ BrowserSkiaGoldPixelDiff::BrowserSkiaGoldPixelDiff() = default; ...@@ -31,8 +31,9 @@ BrowserSkiaGoldPixelDiff::BrowserSkiaGoldPixelDiff() = default;
BrowserSkiaGoldPixelDiff::~BrowserSkiaGoldPixelDiff() = default; BrowserSkiaGoldPixelDiff::~BrowserSkiaGoldPixelDiff() = default;
void BrowserSkiaGoldPixelDiff::Init(views::Widget* widget, void BrowserSkiaGoldPixelDiff::Init(views::Widget* widget,
const std::string& screenshot_prefix) { const std::string& screenshot_prefix,
SkiaGoldPixelDiff::Init(screenshot_prefix); const std::string& corpus) {
SkiaGoldPixelDiff::Init(screenshot_prefix, corpus);
DCHECK(widget); DCHECK(widget);
widget_ = widget; widget_ = widget;
} }
......
...@@ -43,7 +43,12 @@ class BrowserSkiaGoldPixelDiff : public ui::test::SkiaGoldPixelDiff { ...@@ -43,7 +43,12 @@ class BrowserSkiaGoldPixelDiff : public ui::test::SkiaGoldPixelDiff {
// test class name as the prefix. The name will be // test class name as the prefix. The name will be
// |screenshot_prefix| + "_" + |screenshot_name|.' // |screenshot_prefix| + "_" + |screenshot_name|.'
// E.g. 'ToolbarTest_BackButtonHover'. // E.g. 'ToolbarTest_BackButtonHover'.
void Init(views::Widget* widget, const std::string& screenshot_prefix); // corpus The corpus (i.e. result group) that will be used to store the
// result in Gold. If omitted, will default to the generic corpus for
// results from gtest-based tests.
void Init(views::Widget* widget,
const std::string& screenshot_prefix,
const std::string& corpus = std::string());
// Take a screenshot, upload to Skia Gold and compare with the remote // Take a screenshot, upload to Skia Gold and compare with the remote
// golden image. Returns true if the screenshot is the same as the golden // golden image. Returns true if the screenshot is the same as the golden
......
...@@ -200,7 +200,8 @@ void SkiaGoldPixelDiff::InitSkiaGold() { ...@@ -200,7 +200,8 @@ void SkiaGoldPixelDiff::InitSkiaGold() {
ASSERT_EQ(exit_code, 0); ASSERT_EQ(exit_code, 0);
} }
void SkiaGoldPixelDiff::Init(const std::string& screenshot_prefix) { void SkiaGoldPixelDiff::Init(const std::string& screenshot_prefix,
const std::string& corpus) {
auto* cmd_line = base::CommandLine::ForCurrentProcess(); auto* cmd_line = base::CommandLine::ForCurrentProcess();
ASSERT_TRUE(cmd_line->HasSwitch(kBuildRevisionKey)) ASSERT_TRUE(cmd_line->HasSwitch(kBuildRevisionKey))
<< "Missing switch " << kBuildRevisionKey; << "Missing switch " << kBuildRevisionKey;
...@@ -223,6 +224,7 @@ void SkiaGoldPixelDiff::Init(const std::string& screenshot_prefix) { ...@@ -223,6 +224,7 @@ void SkiaGoldPixelDiff::Init(const std::string& screenshot_prefix) {
} }
initialized_ = true; initialized_ = true;
prefix_ = screenshot_prefix; prefix_ = screenshot_prefix;
corpus_ = corpus.length() ? corpus : "gtest-pixeltests";
base::ScopedAllowBlockingForTesting allow_blocking; base::ScopedAllowBlockingForTesting allow_blocking;
base::CreateNewTempDirectory(FILE_PATH_LITERAL("SkiaGoldTemp"), base::CreateNewTempDirectory(FILE_PATH_LITERAL("SkiaGoldTemp"),
&working_dir_); &working_dir_);
...@@ -244,7 +246,7 @@ bool SkiaGoldPixelDiff::UploadToSkiaGoldServer( ...@@ -244,7 +246,7 @@ bool SkiaGoldPixelDiff::UploadToSkiaGoldServer(
base::ScopedAllowBlockingForTesting allow_blocking; base::ScopedAllowBlockingForTesting allow_blocking;
base::CommandLine cmd(GetAbsoluteSrcRelativePath(kSkiaGoldCtl)); base::CommandLine cmd(GetAbsoluteSrcRelativePath(kSkiaGoldCtl));
cmd.AppendSwitchASCII("test-name", remote_golden_image_name); cmd.AppendSwitchASCII("test-name", remote_golden_image_name);
cmd.AppendSwitchASCII("add-test-key", "source_type:gtest-pixeltests"); cmd.AppendSwitchASCII("add-test-key", "source_type:" + corpus_);
cmd.AppendSwitchPath("png-file", local_file_path); cmd.AppendSwitchPath("png-file", local_file_path);
cmd.AppendSwitchPath("work-dir", working_dir_); cmd.AppendSwitchPath("work-dir", working_dir_);
......
...@@ -40,7 +40,11 @@ class SkiaGoldPixelDiff { ...@@ -40,7 +40,11 @@ class SkiaGoldPixelDiff {
// test class name as the prefix. The name will be // test class name as the prefix. The name will be
// |screenshot_prefix| + "_" + |screenshot_name|.' // |screenshot_prefix| + "_" + |screenshot_name|.'
// E.g. 'ToolbarTest_BackButtonHover'. // E.g. 'ToolbarTest_BackButtonHover'.
void Init(const std::string& screenshot_prefix); // corpus The corpus (i.e. result group) that will be used to store the
// result in Gold. If omitted, will default to the generic corpus for
// results from gtest-based tests.
void Init(const std::string& screenshot_prefix,
const std::string& corpus = std::string());
bool CompareScreenshot( bool CompareScreenshot(
const std::string& screenshot_name, const std::string& screenshot_name,
...@@ -65,6 +69,8 @@ class SkiaGoldPixelDiff { ...@@ -65,6 +69,8 @@ class SkiaGoldPixelDiff {
bool initialized_ = false; bool initialized_ = false;
// Use luci auth on bots. Don't use luci auth for local development. // Use luci auth on bots. Don't use luci auth for local development.
bool luci_auth_ = true; bool luci_auth_ = true;
// Which corpus in the instance to associate results with.
std::string corpus_;
// Build revision. This is only used for CI run. // Build revision. This is only used for CI run.
std::string build_revision_; std::string build_revision_;
// The following 3 members are for tryjob run. // The following 3 members are for tryjob run.
......
...@@ -162,5 +162,44 @@ TEST_F(SkiaGoldPixelDiffTest, SobelMatching) { ...@@ -162,5 +162,44 @@ TEST_F(SkiaGoldPixelDiffTest, SobelMatching) {
EXPECT_TRUE(ret); EXPECT_TRUE(ret);
} }
TEST_F(SkiaGoldPixelDiffTest, DefaultCorpus) {
SkBitmap bitmap;
SkImageInfo info =
SkImageInfo::Make(10, 10, SkColorType::kBGRA_8888_SkColorType,
SkAlphaType::kPremul_SkAlphaType);
bitmap.allocPixels(info, 10 * 4);
MockSkiaGoldPixelDiff mock_pixel;
EXPECT_CALL(mock_pixel, LaunchProcess(_)).Times(AnyNumber());
EXPECT_CALL(
mock_pixel,
LaunchProcess(AllOf(Property(
&base::CommandLine::GetCommandLineString,
HasSubstr(FILE_PATH_LITERAL("source_type:gtest-pixeltests"))))))
.Times(1);
mock_pixel.Init("Prefix");
bool ret = mock_pixel.CompareScreenshot("test", bitmap);
EXPECT_TRUE(ret);
}
TEST_F(SkiaGoldPixelDiffTest, ExplicitCorpus) {
SkBitmap bitmap;
SkImageInfo info =
SkImageInfo::Make(10, 10, SkColorType::kBGRA_8888_SkColorType,
SkAlphaType::kPremul_SkAlphaType);
bitmap.allocPixels(info, 10 * 4);
MockSkiaGoldPixelDiff mock_pixel;
EXPECT_CALL(mock_pixel, LaunchProcess(_)).Times(AnyNumber());
EXPECT_CALL(mock_pixel,
LaunchProcess(AllOf(Property(
&base::CommandLine::GetCommandLineString,
HasSubstr(FILE_PATH_LITERAL("source_type:corpus"))))))
.Times(1);
mock_pixel.Init("Prefix", "corpus");
bool ret = mock_pixel.CompareScreenshot("test", bitmap);
EXPECT_TRUE(ret);
}
} // namespace test } // namespace test
} // namespace ui } // namespace ui
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment