[focusgroup] Add PrintTo helper for logging FocusgroupFlags [chromium/src : main]
Jacques Newman (Gerrit)
Jacques Newman added 1 comment![]( "Open in Gerrit")
CORE_EXPORT String FocusgroupFlagsToStringForTesting(FocusgroupFlags flags);I found this export was needed to build some configurations of blink_unittests, let me know if there is a better pattern I can use here, as subsequent changes add more helpers that run into the same issue where testing is the only place where they will be called outside of core. (Of course, this one is only ever called in test).
Related details
- Mason Freed
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mason Freed (Gerrit)
Mason Freed voted and added 3 comments![]( "Open in Gerrit")
Votes added by Mason Freed
| Code-Review | +1 |
3 comments
LGTM, but +pdr@ for more advice on exposing debug strings.
focusgroup::FocusgroupFlagsToStringForTesting(FocusgroupFlags::kNone));A test for the test strings function. I like it. 😊
CORE_EXPORT String FocusgroupFlagsToStringForTesting(FocusgroupFlags flags);I found this export was needed to build some configurations of blink_unittests, let me know if there is a better pattern I can use here, as subsequent changes add more helpers that run into the same issue where testing is the only place where they will be called outside of core. (Of course, this one is only ever called in test).
+pdr@ who I think has added some similar test-only helpers in the past. This seems ok to me, but it's hard to find a similar pattern to follow in the codebase.
Related details
- Jacques Newman
- Philip Rogers
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Philip Rogers (Gerrit)
Philip Rogers voted and added 1 comment![]( "Open in Gerrit")
Votes added by Philip Rogers
| Code-Review | +1 |
1 comment
CORE_EXPORT String FocusgroupFlagsToStringForTesting(FocusgroupFlags flags);Mason FreedI found this export was needed to build some configurations of blink_unittests, let me know if there is a better pattern I can use here, as subsequent changes add more helpers that run into the same issue where testing is the only place where they will be called outside of core. (Of course, this one is only ever called in test).
+pdr@ who I think has added some similar test-only helpers in the past. This seems ok to me, but it's hard to find a similar pattern to follow in the codebase.
This seems fine to me.
Related details
- Jacques Newman
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jacques Newman (Gerrit)
Jacques Newman voted Commit-Queue+2![]( "Open in Gerrit")
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
[focusgroup] Add PrintTo helper for logging FocusgroupFlags
Adds helper to make FocusgroupFlags human-readable for tests and debug
logging.
This change also updates the FocusgroupFlags parsing unittests to
prefer EXPECT_* over ASSERT_* for situations where it would be
possible for the test to continue.
- M third_party/blink/renderer/core/dom/element_test.cc
- M third_party/blink/renderer/core/dom/focusgroup_flags.cc
- M third_party/blink/renderer/core/dom/focusgroup_flags.h
Code-Review: +1 by Mason Freed, +1 by Philip Rogers
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |