[Test Automation] Add temporary values that can be shared between steps [chromium/src : main]

[gwsq (Gerrit)]

Attention needed from Christopher Grant

Message from gwsq

Reviewer source(s):
cjg...@chromium.org is from context(googleclient/chrome/chromium_gwsq/team/topchrome/config.gwsq)

Open in Gerrit

Related details

Attention is currently required from:

• Christopher Grant

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I1adc413b9297435c5b90734b5f41f55f4671e24e

Gerrit-Change-Number: 7693352

Gerrit-PatchSet: 3

Gerrit-Owner: Dana Fried <dfr...@chromium.org>

Gerrit-Reviewer: Christopher Grant <cjg...@chromium.org>

Gerrit-Reviewer: Dana Fried <dfr...@chromium.org>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: TopChrome team GWSQ <top-chr...@google.com>

Gerrit-CC: chromium...@chromium.org

Gerrit-CC: gwsq

Gerrit-Attention: Christopher Grant <cjg...@chromium.org>

Gerrit-Comment-Date: Tue, 24 Mar 2026 13:05:05 +0000

Gerrit-HasComments: No

Gerrit-Has-Labels: No

[Christopher Grant (Gerrit)]

Attention needed from Dana Fried

Christopher Grant added 4 comments

Patchset-level comments

File-level comment, Patchset 3 (Latest):

Christopher Grant . resolved

Thanks for adding this - it addresses what feels like a primary oddity and pain point in Kombucha.

File ui/base/interaction/interactive_test.h

Line 713, Patchset 3 (Latest): void FreeTemporaryValue(TemporaryIdentifier<V> variable) {

Christopher Grant . unresolved

Naming: Why Free vs. Reset or Release? Is this a homage to C? Modern C++ coders are conditioned to bristle at the word "free".

File ui/base/interaction/interactive_test_temporary.h

Line 19, Patchset 3 (Latest):DECLARE_UNIQUE_IDENTIFIER_TYPE(InteractiveTestTemporaryIdentifier);

Christopher Grant . unresolved

With anything Kombucha, the API tends to be clean and simple, but under the hood, there's a bunch of complexity. Can you include class-level comments on the pieces in here, to cover things like (eg) why does an identifier need to be templated? Doesn't have to be a novel, but "some" comments would be better than none, for someone who wants to dig into this later.

File ui/base/interaction/interactive_test_unittest.cc

Line 72, Patchset 3 (Latest):INTERACTIVE_TEST_TEMPORARY_VALUE(int, kIntVariable);

Christopher Grant . unresolved

Naming nit: After reading this CL a couple times, why "temporary"? Everything's temporary in a context like this. My mind went for just INTERACTIVE_TEST_VALUE(), with SetTestValue() and GetTestValue() for accessing them. Or even SetValue() and GetValue(). Would that be too ambiguous with base::Value? What made you use "Temporary"?

Open in Gerrit

Related details

Attention is currently required from:

• Dana Fried

Submit Requirements:

• Code-Coverage
• Code-Owners
• Code-Review

• No-Unresolved-Comments

• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I1adc413b9297435c5b90734b5f41f55f4671e24e

Gerrit-Change-Number: 7693352

Gerrit-PatchSet: 3

Gerrit-Owner: Dana Fried <dfr...@chromium.org>

Gerrit-Reviewer: Christopher Grant <cjg...@chromium.org>

Gerrit-Reviewer: Dana Fried <dfr...@chromium.org>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: TopChrome team GWSQ <top-chr...@google.com>

Gerrit-CC: chromium...@chromium.org

Gerrit-CC: gwsq

Gerrit-Attention: Dana Fried <dfr...@chromium.org>

Gerrit-Comment-Date: Tue, 24 Mar 2026 14:23:47 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Christopher Grant (Gerrit)]

Attention needed from Dana Fried

Christopher Grant added 1 comment

File ui/base/interaction/interactive_test.h

Line 713, Patchset 3 (Latest): void FreeTemporaryValue(TemporaryIdentifier<V> variable) {

Christopher Grant . unresolved

Naming: Why Free vs. Reset or Release? Is this a homage to C? Modern C++ coders are conditioned to bristle at the word "free".

Christopher Grant

BTW, this doesn't have to change; just food for thought.

Gerrit-Comment-Date: Tue, 24 Mar 2026 15:21:17 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

Comment-In-Reply-To: Christopher Grant <cjg...@chromium.org>