New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 872748 link

Starred by 1 user
Status: Fixed
Owner:
█ joenotcharles@chromium.org
Last visit > 30 days ago
Closed: Aug 15
Cc:
vakh@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug



Sign in to add a comment

Find out why GetParamNameForTests doesn't work with ExecutionMode enum

Project Member Reported by joenotcharles@chromium.org, Aug 9 
I want to make another try to figure out what's wrong with this, because I like GetParamNameForTest, but if I can't figure it out I'll decide it's too fragile and remove the whole thing.

The root cause is that in a debug build, the compiler insists on choosing

template <typename T>
void PrintTo(const T& value, ::std::ostream* os) {

Defined in third_party/googletest/src/googletest/include/gtest/gtest-printers.h, with T as chrome_cleaner::ExecutionMode (an enum class defined in components/chrome_cleaner/public/constants.h) even when

void PrintTo(const chrome_cleaner::ExecutionMode& mode, ::std::ostream* os) {

Is defined in chrome/chrome_cleaner/test/test_name_helper.h

But only in debug mode. Release is fine.

Comment 1 by vakh@chromium.org, Aug 10

Labels: SafeBrowsing-Triaged
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 10 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/29182424f4d1c29a0914e09916a198f4d1a4897a

commit 29182424f4d1c29a0914e09916a198f4d1a4897a
Author: Joe Mason <joenotcharles@chromium.org>
Date: Fri Aug 10 19:24:16 2018

Re-enable pretty-printed ExecutionMode test names

Fix ODR violation with ExecutionMode by declaring the streaming operator in the
same header as the type, and add back the GetParamNameForTest calls that use
that operator.

Also add a streaming operator for ChromePromptValue so that all types declared
in constants.h can be pretty-printed.

R=proberge

Bug:  872748 
Change-Id: Idfe3f6044f97b003f17d83c6985d442f9b4103f6
Reviewed-on: https://chromium-review.googlesource.com/1170852
Commit-Queue: Joe Mason <joenotcharles@chromium.org>
Reviewed-by: proberge <proberge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582285}
[modify] https://crrev.com/29182424f4d1c29a0914e09916a198f4d1a4897a/chrome/chrome_cleaner/logging/BUILD.gn
[modify] https://crrev.com/29182424f4d1c29a0914e09916a198f4d1a4897a/chrome/chrome_cleaner/logging/cleaner_logging_service_unittest.cc
[modify] https://crrev.com/29182424f4d1c29a0914e09916a198f4d1a4897a/chrome/chrome_cleaner/settings/BUILD.gn
[modify] https://crrev.com/29182424f4d1c29a0914e09916a198f4d1a4897a/chrome/chrome_cleaner/settings/cleaner_settings_unittest.cc
[modify] https://crrev.com/29182424f4d1c29a0914e09916a198f4d1a4897a/chrome/chrome_cleaner/test/BUILD.gn
[delete] https://crrev.com/56d2155d33fd78eac37ae544ac86c750d188f0f4/chrome/chrome_cleaner/test/test_name_helper.cc
[modify] https://crrev.com/29182424f4d1c29a0914e09916a198f4d1a4897a/chrome/chrome_cleaner/test/test_name_helper.h
[modify] https://crrev.com/29182424f4d1c29a0914e09916a198f4d1a4897a/chrome/test/BUILD.gn
[modify] https://crrev.com/29182424f4d1c29a0914e09916a198f4d1a4897a/components/chrome_cleaner/public/constants/BUILD.gn
[modify] https://crrev.com/29182424f4d1c29a0914e09916a198f4d1a4897a/components/chrome_cleaner/public/constants/constants.h
[add] https://crrev.com/29182424f4d1c29a0914e09916a198f4d1a4897a/components/chrome_cleaner/public/constants/constants_test_support.cc
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 10 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/da2cbfa6d74f34482c227e4715bc0b03c3298f01

commit da2cbfa6d74f34482c227e4715bc0b03c3298f01
Author: Joe Mason <joenotcharles@chromium.org>
Date: Fri Aug 10 21:34:05 2018

Make ExecutionMode pretty-printer available for CHECK outside tests

R=proberge

Bug:  872748 
Change-Id: I42ae9acf6bd189ef45543ddfa15521262c56b95f
Reviewed-on: https://chromium-review.googlesource.com/1171549
Reviewed-by: proberge <proberge@chromium.org>
Commit-Queue: Joe Mason <joenotcharles@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582343}
[modify] https://crrev.com/da2cbfa6d74f34482c227e4715bc0b03c3298f01/chrome/chrome_cleaner/logging/BUILD.gn
[modify] https://crrev.com/da2cbfa6d74f34482c227e4715bc0b03c3298f01/chrome/chrome_cleaner/settings/BUILD.gn
[modify] https://crrev.com/da2cbfa6d74f34482c227e4715bc0b03c3298f01/chrome/chrome_cleaner/test/BUILD.gn
[modify] https://crrev.com/da2cbfa6d74f34482c227e4715bc0b03c3298f01/chrome/test/BUILD.gn
[modify] https://crrev.com/da2cbfa6d74f34482c227e4715bc0b03c3298f01/components/chrome_cleaner/public/constants/BUILD.gn
[modify] https://crrev.com/da2cbfa6d74f34482c227e4715bc0b03c3298f01/components/chrome_cleaner/public/constants/constants.cc
[modify] https://crrev.com/da2cbfa6d74f34482c227e4715bc0b03c3298f01/components/chrome_cleaner/public/constants/constants.h
[delete] https://crrev.com/7f7fafa75c2080ef3a234472c8926be2ade8b04b/components/chrome_cleaner/public/constants/constants_test_support.cc
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 14 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a4b6ac827d08c1281ca92b8fbe9f9cf22c46f16a

commit a4b6ac827d08c1281ca92b8fbe9f9cf22c46f16a
Author: Joe Mason <joenotcharles@chromium.org>
Date: Tue Aug 14 15:38:16 2018

Add chrome_cleaner/ui dir

Also add missed chrome_utils unit tests to test list.

R=proberge

Bug:  872748 
Change-Id: Ica0f2a33768bbe4f9cfb2ff5842e47a1b17e53a0
Reviewed-on: https://chromium-review.googlesource.com/1173315
Reviewed-by: Chris Sharp <csharp@chromium.org>
Commit-Queue: Joe Mason <joenotcharles@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582920}
[modify] https://crrev.com/a4b6ac827d08c1281ca92b8fbe9f9cf22c46f16a/chrome/chrome_cleaner/BUILD.gn
[add] https://crrev.com/a4b6ac827d08c1281ca92b8fbe9f9cf22c46f16a/chrome/chrome_cleaner/ui/BUILD.gn
[add] https://crrev.com/a4b6ac827d08c1281ca92b8fbe9f9cf22c46f16a/chrome/chrome_cleaner/ui/chrome_proxy_main_dialog.cc
[add] https://crrev.com/a4b6ac827d08c1281ca92b8fbe9f9cf22c46f16a/chrome/chrome_cleaner/ui/chrome_proxy_main_dialog.h
[add] https://crrev.com/a4b6ac827d08c1281ca92b8fbe9f9cf22c46f16a/chrome/chrome_cleaner/ui/chrome_proxy_main_dialog_unittest.cc
[add] https://crrev.com/a4b6ac827d08c1281ca92b8fbe9f9cf22c46f16a/chrome/chrome_cleaner/ui/main_dialog_api.cc
[add] https://crrev.com/a4b6ac827d08c1281ca92b8fbe9f9cf22c46f16a/chrome/chrome_cleaner/ui/main_dialog_api.h
[add] https://crrev.com/a4b6ac827d08c1281ca92b8fbe9f9cf22c46f16a/chrome/chrome_cleaner/ui/mock_main_dialog_delegate.cc
[add] https://crrev.com/a4b6ac827d08c1281ca92b8fbe9f9cf22c46f16a/chrome/chrome_cleaner/ui/mock_main_dialog_delegate.h
[add] https://crrev.com/a4b6ac827d08c1281ca92b8fbe9f9cf22c46f16a/chrome/chrome_cleaner/ui/silent_main_dialog.cc
[add] https://crrev.com/a4b6ac827d08c1281ca92b8fbe9f9cf22c46f16a/chrome/chrome_cleaner/ui/silent_main_dialog.h
[add] https://crrev.com/a4b6ac827d08c1281ca92b8fbe9f9cf22c46f16a/chrome/chrome_cleaner/ui/silent_main_dialog_unittest.cc

Comment 5 by joenotcharles@chromium.org, Aug 15

Status: Fixed (was: Untriaged)
►

Sign in to add a comment