[remoting]: Fix Yoda-style asserts in host tests [chromium/src : main]

[Joe Downing (Gerrit)]

Attention needed from Yuwei Huang

Joe Downing added 1 comment

Patchset-level comments

File-level comment, Patchset 4 (Latest):

Joe Downing . resolved

PTAL!

Open in Gerrit

Related details

Attention is currently required from:

• Yuwei Huang

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: Id46e4ce129baeb27f43d534d4251d6418791a6f9

Gerrit-Change-Number: 7721702

Gerrit-PatchSet: 4

Gerrit-Owner: Joe Downing <joe...@chromium.org>

Gerrit-Reviewer: Joe Downing <joe...@chromium.org>

Gerrit-Reviewer: Yuwei Huang <yuw...@chromium.org>

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

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

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

Gerrit-Attention: Yuwei Huang <yuw...@chromium.org>

Gerrit-Comment-Date: Wed, 01 Apr 2026 23:53:47 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Yuwei Huang (Gerrit)]

Attention needed from Joe Downing

Yuwei Huang voted and added 5 comments

Votes added by Yuwei Huang

Code-Review

+1

5 comments

Patchset-level comments

File-level comment, Patchset 4 (Latest):

Yuwei Huang . resolved

LGTM w/ AI comments that I've confirmed.

File remoting/host/heartbeat_sender_unittest.cc

Line 76, Patchset 4 (Latest): ASSERT_EQ(options.is_initial_heartbeat, is_initial_heartbeat);

Yuwei Huang . unresolved

`is_initial_heartbeat` is the actual value passed into the lambda, and `options.is_initial_heartbeat` is the configured expected value. This was already `(actual, expected)`, so flipping it here accidentally made it `(expected, actual)`!

File remoting/host/linux/gdbus_connection_ref_unittest.cc

Line 136, Patchset 4 (Latest): EXPECT_EQ(GVariantRef<"v">::From(gvariant::Boxed(value)),

Yuwei Huang . unresolved

Missed one! `GVariantRef<"v">::From(gvariant::Boxed(value))` is the expected value. (Also check line 245 below).

File remoting/host/linux/gvariant_ref_unittest.cc

Line 148, Patchset 4 (Latest): GVariantParse<"ms">("just 'apple'").Into<std::optional<std::string>>());

Yuwei Huang . unresolved

Missed one! `std::optional(std::string("apple"))` is the expected value, and `GVariantParse...` is the actual. There are actually several missed flips throughout this file.

File remoting/host/resizing_host_observer_unittest.cc

Line 358, Patchset 4 (Latest): EXPECT_EQ(MakeResolution(100, 100),

Yuwei Huang . unresolved

Missed one! `MakeResolution(100, 100)` is the expected value here.

Open in Gerrit

Related details

Attention is currently required from:

• Joe Downing

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: Id46e4ce129baeb27f43d534d4251d6418791a6f9

Gerrit-Change-Number: 7721702

Gerrit-PatchSet: 4

Gerrit-Owner: Joe Downing <joe...@chromium.org>

Gerrit-Reviewer: Joe Downing <joe...@chromium.org>

Gerrit-Reviewer: Yuwei Huang <yuw...@chromium.org>

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

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

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

Gerrit-Attention: Joe Downing <joe...@chromium.org>

Gerrit-Comment-Date: Thu, 02 Apr 2026 00:12:20 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: Yes

[Joe Downing (Gerrit)]

Joe Downing voted and added 5 comments

Votes added by Joe Downing

Commit-Queue

+2

5 comments

Patchset-level comments

File-level comment, Patchset 5 (Latest):

Joe Downing . resolved

Thanks!

File remoting/host/heartbeat_sender_unittest.cc

Line 76, Patchset 4: ASSERT_EQ(options.is_initial_heartbeat, is_initial_heartbeat);

Yuwei Huang . resolved

`is_initial_heartbeat` is the actual value passed into the lambda, and `options.is_initial_heartbeat` is the configured expected value. This was already `(actual, expected)`, so flipping it here accidentally made it `(expected, actual)`!

Joe Downing

Ah you're right, this is a tricky one. I need to flip line 80 as well then.

File remoting/host/linux/gdbus_connection_ref_unittest.cc

Line 136, Patchset 4: EXPECT_EQ(GVariantRef<"v">::From(gvariant::Boxed(value)),

Yuwei Huang . resolved

Missed one! `GVariantRef<"v">::From(gvariant::Boxed(value))` is the expected value. (Also check line 245 below).

Joe Downing

Oh interesting, thanks!

File remoting/host/linux/gvariant_ref_unittest.cc

Line 148, Patchset 4: GVariantParse<"ms">("just 'apple'").Into<std::optional<std::string>>());

Yuwei Huang . resolved

Missed one! `std::optional(std::string("apple"))` is the expected value, and `GVariantParse...` is the actual. There are actually several missed flips throughout this file.

Joe Downing

Done!

File remoting/host/resizing_host_observer_unittest.cc

Line 358, Patchset 4: EXPECT_EQ(MakeResolution(100, 100),

Yuwei Huang . resolved

Missed one! `MakeResolution(100, 100)` is the expected value here.

Joe Downing

Done

Open in Gerrit

Related details

Attention set is empty

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: Id46e4ce129baeb27f43d534d4251d6418791a6f9

Gerrit-Change-Number: 7721702

Gerrit-PatchSet: 5

Gerrit-Owner: Joe Downing <joe...@chromium.org>

Gerrit-Reviewer: Joe Downing <joe...@chromium.org>

Gerrit-Reviewer: Yuwei Huang <yuw...@chromium.org>

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

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

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

Gerrit-Comment-Date: Thu, 02 Apr 2026 16:44:42 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: Yes

Comment-In-Reply-To: Yuwei Huang <yuw...@chromium.org>

[Chromium LUCI CQ (Gerrit)]

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

4 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: remoting/host/linux/gvariant_ref_unittest.cc
Insertions: 36, Deletions: 35.

@@ -144,11 +144,10 @@
       base::ok(GVariantParse<"mn">("just 539")));
 
   EXPECT_EQ(
-      std::optional(std::string("apple")),
-      GVariantParse<"ms">("just 'apple'").Into<std::optional<std::string>>());
-  EXPECT_EQ(
-      std::optional<std::uint32_t>(),
-      GVariantParse<"mu">("nothing").Into<std::optional<std::uint32_t>>());
+      GVariantParse<"ms">("just 'apple'").Into<std::optional<std::string>>(),
+      std::optional(std::string("apple")));
+  EXPECT_EQ(GVariantParse<"mu">("nothing").Into<std::optional<std::uint32_t>>(),
+            std::optional<std::uint32_t>());
   EXPECT_EQ((GVariantParse<"ms", "*">("just 'peach'")
                  .TryInto<std::optional<std::string>>()),
             base::ok(std::optional(std::string("peach"))));
@@ -183,9 +182,9 @@
   EXPECT_EQ((GVariantParse<"ad", "*">("[]").TryInto<std::vector<double>>()),
             base::ok(std::vector<double>()));
   EXPECT_EQ(
-      (std::vector<std::string>{"cabbage", "broccoli", "kale", "cauliflower"}),
       GVariantParse<"as">("['cabbage', 'broccoli', 'kale', 'cauliflower']")
-          .Into<std::vector<std::string>>());
+          .Into<std::vector<std::string>>(),
+      (std::vector<std::string>{"cabbage", "broccoli", "kale", "cauliflower"}));
 
   EXPECT_FALSE((GVariantParse<"i", "*">("7")
                     .TryInto<std::vector<std::int32_t>>()
@@ -218,10 +217,10 @@
   EXPECT_EQ((GVariantParse<"a{by}", "*">("{}")
                  .TryInto<std::map<bool, std::uint8_t>>()),
             base::ok(std::map<bool, std::uint8_t>()));
-  EXPECT_EQ(
-      (std::map<std::int32_t, double>{{1, 1}, {2, 0.5}, {4, 0.25}, {8, 0.125}}),
-      (GVariantParse<"a{id}">("{1: 1, 2: 0.5, 4: 0.25, 8: 0.125}")
-           .Into<std::map<int32_t, double>>()));
+  EXPECT_EQ((GVariantParse<"a{id}">("{1: 1, 2: 0.5, 4: 0.25, 8: 0.125}")
+                 .Into<std::map<int32_t, double>>()),
+            (std::map<std::int32_t, double>{
+                {1, 1}, {2, 0.5}, {4, 0.25}, {8, 0.125}}));
 
   EXPECT_FALSE((GVariantParse<"i", "*">("-3")
                     .TryInto<std::map<std::uint32_t, std::uint8_t>>()
@@ -254,9 +253,9 @@
   EXPECT_EQ((GVariantParse<"{by}", "*">("{true, 9}")
                  .TryInto<std::pair<bool, std::uint8_t>>()),
             base::ok(std::pair<bool, std::uint8_t>(true, 9)));
-  EXPECT_EQ((std::pair<double, std::string>(6.5, "poof")),
-            (GVariantParse<"{ds}">("{6.5, 'poof'}")
-                 .Into<std::pair<double, std::string>>()));
+  EXPECT_EQ((GVariantParse<"{ds}">("{6.5, 'poof'}")
+                 .Into<std::pair<double, std::string>>()),
+            (std::pair<double, std::string>(6.5, "poof")));
 
   EXPECT_FALSE((GVariantParse<"{uu}", "*">("{5, 7}")
                     .TryInto<std::pair<std::uint32_t, std::uint8_t>>()
@@ -282,10 +281,10 @@
 
   EXPECT_EQ((GVariantParse<"()", "*">("()").TryInto<std::tuple<>>()),
             base::ok(std::tuple()));
-  EXPECT_EQ(
-      std::tuple(std::string("fancy"), false, double{-6.75}, std::int64_t{512}),
-      (GVariantParse<"(sbdx)">("('fancy', false, -6.75, 512)")
-           .Into<std::tuple<std::string, bool, double, std::int64_t>>()));
+  EXPECT_EQ((GVariantParse<"(sbdx)">("('fancy', false, -6.75, 512)")
+                 .Into<std::tuple<std::string, bool, double, std::int64_t>>()),
+            std::tuple(std::string("fancy"), false, double{-6.75},
+                       std::int64_t{512}));
 
   EXPECT_FALSE((GVariantParse<"i", "*">("4")
                     .TryInto<std::tuple<std::int32_t>>()
@@ -332,19 +331,21 @@
   EXPECT_EQ(GVariantParse<"ms">("just 'delicata squash'"),
             GVariantFrom(Mixed(std::in_place_index<2>, "delicata squash")));
 
-  EXPECT_EQ(Strings(std::in_place_index<1>, "eggplant"),
-            GVariantParse<"s">("'eggplant'").Into<Strings>());
+  EXPECT_EQ(GVariantParse<"s">("'eggplant'").Into<Strings>(),
+            Strings(std::in_place_index<1>, "eggplant"));
 
-  EXPECT_EQ(BasicArrays(std::in_place_index<2>, std::vector{true, false, true}),
-            GVariantParse<"ab">("[true, false, true]").Into<BasicArrays>());
+  EXPECT_EQ(
+      GVariantParse<"ab">("[true, false, true]").Into<BasicArrays>(),
+      BasicArrays(std::in_place_index<2>, std::vector{true, false, true}));
 
   EXPECT_EQ((GVariantParse<"y", "*">("12").TryInto<Mixed>()),
             base::ok(Mixed(std::in_place_index<0>, 12)));
-  EXPECT_EQ(Mixed(std::in_place_index<1>, 9.75),
-            GVariantParse<"d">("9.75").Into<GVariantRef<"?">>().Into<Mixed>());
+  EXPECT_EQ(GVariantParse<"d">("9.75").Into<GVariantRef<"?">>().Into<Mixed>(),
+            Mixed(std::in_place_index<1>, 9.75));
   EXPECT_EQ(
-      Mixed(std::in_place_index<3>, GVariantRef<"?">::From(std::int32_t{93})),
-      GVariantParse<"i">("93").Into<Mixed>());
+
+      GVariantParse<"i">("93").Into<Mixed>(),
+      Mixed(std::in_place_index<3>, GVariantRef<"?">::From(std::int32_t{93})));
 
   // Doesn't match type string.
   EXPECT_FALSE((GVariantParse<"i", "*">("16").TryInto<Strings>().has_value()));
@@ -418,11 +419,11 @@
   EXPECT_EQ(GVariantParse<"v">("<['chirp', 'whistle']>"),
             GVariantFrom(BoxedRef(value)));
 
-  EXPECT_EQ((Boxed<std::variant<std::uint16_t, std::string>>{"boop"}),
-            (GVariantParse<"v">("<'boop'>")
-                 .TryInto<Boxed<std::variant<std::uint16_t, std::string>>>()));
-  EXPECT_EQ(Boxed{GVariantFrom(std::int64_t{-204})},
-            GVariantParse<"v">("<int64 -204>").Into<Boxed<GVariantRef<>>>());
+  EXPECT_EQ((GVariantParse<"v">("<'boop'>")
+                 .TryInto<Boxed<std::variant<std::uint16_t, std::string>>>()),
+            (Boxed<std::variant<std::uint16_t, std::string>>{"boop"}));
+  EXPECT_EQ(GVariantParse<"v">("<int64 -204>").Into<Boxed<GVariantRef<>>>(),
+            Boxed{GVariantFrom(std::int64_t{-204})});
 
   EXPECT_FALSE((GVariantParse<"i", "*">("-27")
                     .TryInto<Boxed<std::int32_t>>()
@@ -486,8 +487,8 @@
   EXPECT_EQ(GVariantParse<"o">("'/valid/path'"),
             GVariantFrom(ObjectPathCStr(path.value())));
 
-  EXPECT_EQ(path.value(),
-            GVariantParse<"o">("'/valid/path'").Into<ObjectPath>());
+  EXPECT_EQ(GVariantParse<"o">("'/valid/path'").Into<ObjectPath>(),
+            path.value());
 
   EXPECT_EQ(GVariantParse<"o">("'/path/constant'"),
             GVariantFrom(ObjectPathCStr("/path/constant")));
@@ -507,8 +508,8 @@
   EXPECT_EQ(GVariantParse<"g">("'goodsig'"),
             GVariantFrom(TypeSignatureCStr(signature.value())));
 
-  EXPECT_EQ(signature.value(),
-            GVariantParse<"g">("'goodsig'").Into<TypeSignature>());
+  EXPECT_EQ(GVariantParse<"g">("'goodsig'").Into<TypeSignature>(),
+            signature.value());
 
   EXPECT_EQ(GVariantParse<"g">("'gonstsig'"),
             GVariantFrom(TypeSignatureCStr("gonstsig")));
```
```
The name of the file: remoting/host/linux/gdbus_connection_ref_unittest.cc
Insertions: 4, Deletions: 4.

@@ -133,8 +133,8 @@
   auto [interface_name, changed_properties, invalidated_properties] =
       change_signal.Take();
   EXPECT_EQ(interface_name.string_view(), test_interface::Name::kInterfaceName);
-  EXPECT_EQ(GVariantRef<"v">::From(gvariant::Boxed(value)),
-            changed_properties.LookUp(test_interface::Name::kPropertyName));
+  EXPECT_EQ(changed_properties.LookUp(test_interface::Name::kPropertyName),
+            GVariantRef<"v">::From(gvariant::Boxed(value)));
 }
 
 TEST_F(GDBusConnectionRefTest, SignalSubscribe) {
@@ -242,8 +242,8 @@
         GVariantRef<org_freedesktop_DBus_Properties::PropertiesChanged::kType>::
             TryFrom(arguments);
     ASSERT_TRUE(typed_args.has_value());
-    EXPECT_EQ(GVariantRef<"v">::From(gvariant::Boxed(prop_value)),
-              typed_args->get<1>().LookUp(test_interface::Name::kPropertyName));
+    EXPECT_EQ(typed_args->get<1>().LookUp(test_interface::Name::kPropertyName),
+              GVariantRef<"v">::From(gvariant::Boxed(prop_value)));
   }
 }
 
```
```
The name of the file: remoting/host/heartbeat_sender_unittest.cc
Insertions: 2, Deletions: 2.

@@ -73,11 +73,11 @@
   return [=](bool is_initial_heartbeat, std::optional<std::string> signaling_id,
              std::optional<std::string> offline_reason,
              HeartbeatServiceClient::HeartbeatResponseCallback callback) {
-    ASSERT_EQ(options.is_initial_heartbeat, is_initial_heartbeat);
+    ASSERT_EQ(is_initial_heartbeat, options.is_initial_heartbeat);
     if (options.host_offline_reason.empty()) {
       ASSERT_FALSE(offline_reason);
     } else {
-      ASSERT_EQ(options.host_offline_reason, *offline_reason);
+      ASSERT_EQ(*offline_reason, options.host_offline_reason);
     }
 
     base::TimeDelta wait_interval = base::Seconds(kGoodIntervalSeconds);
```
```
The name of the file: remoting/host/resizing_host_observer_unittest.cc
Insertions: 2, Deletions: 2.

@@ -355,8 +355,8 @@
   EXPECT_EQ(GetBestResolution(MakeResolution(200, 200)),
             MakeResolution(100, 100));
   clock_.Advance(base::Milliseconds(99));
-  EXPECT_EQ(MakeResolution(100, 100),
-            GetBestResolution(MakeResolution(300, 300)));
+  EXPECT_EQ(GetBestResolution(MakeResolution(300, 300)),
+            MakeResolution(100, 100));
   clock_.Advance(base::Milliseconds(1));
 
   // Due to the kMinimumResizeIntervalMs constant in resizing_host_observer.cc,
```

Change information

Commit message:

[remoting]: Fix Yoda-style asserts in host tests

This CL flips the arguments of EXPECT_EQ/ASSERT_EQ in
remoting/host/*test.cc from (expected, actual) to the more natural
(actual, expected) format.

Also fixed a few minor issues I found along the way.

Change-Id: Id46e4ce129baeb27f43d534d4251d6418791a6f9

Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7721702

Reviewed-by: Yuwei Huang <yuw...@chromium.org>

Commit-Queue: Joe Downing <joe...@chromium.org>

Cr-Commit-Position: refs/heads/main@{#1609295}

Files:

• M remoting/host/audio_capturer_chromeos_unittest.cc
• M remoting/host/audio_silence_detector_unittest.cc
• M remoting/host/backoff_timer_unittest.cc
• M remoting/host/chromeos/audio_helper_chromeos_impl_unittest.cc
• M remoting/host/chromoting_host_unittest.cc
• M remoting/host/client_session_unittest.cc
• M remoting/host/daemon_process_unittest.cc
• M remoting/host/evaluate_capability_unittest.cc
• M remoting/host/file_transfer/buffered_file_writer_unittest.cc
• M remoting/host/file_transfer/file_transfer_message_handler_unittest.cc
• M remoting/host/file_transfer/local_file_operations_unittest.cc
• M remoting/host/ftl_echo_message_listener_unittest.cc
• M remoting/host/ftl_signaling_connector_unittest.cc
• M remoting/host/heartbeat_sender_unittest.cc
• M remoting/host/host_extension_session_manager_unittest.cc
• M remoting/host/it2me/it2me_host_unittest.cc
• M remoting/host/it2me/it2me_native_messaging_host_unittest.cc
• M remoting/host/linux/audio_pipe_reader_unittest.cc
• M remoting/host/linux/certificate_watcher_unittest.cc
• M remoting/host/linux/gdbus_connection_ref_unittest.cc
• M remoting/host/linux/gnome_display_config_dbus_client_unittest.cc
• M remoting/host/linux/gvariant_ref_unittest.cc
• M remoting/host/linux/unicode_to_keysym_unittest.cc
• M remoting/host/linux/x11_character_injector_unittest.cc
• M remoting/host/linux/x_server_clipboard_unittest.cc
• M remoting/host/mouse_cursor_monitor_proxy_unittest.cc
• M remoting/host/mouse_shape_pump_unittest.cc
• M remoting/host/native_messaging/native_messaging_reader_unittest.cc
• M remoting/host/native_messaging/native_messaging_writer_unittest.cc
• M remoting/host/pin_hash_unittest.cc
• M remoting/host/policy_watcher_unittest.cc
• M remoting/host/remote_open_url/remote_open_url_message_handler_unittest.cc
• M remoting/host/resizing_host_observer_unittest.cc
• M remoting/host/resources_unittest.cc
• M remoting/host/security_key/security_key_auth_handler_mojo_unittest.cc
• M remoting/host/security_key/security_key_auth_handler_posix_unittest.cc
• M remoting/host/security_key/security_key_message_handler_unittest.cc
• M remoting/host/security_key/security_key_message_reader_impl_unittest.cc
• M remoting/host/security_key/security_key_message_writer_impl_unittest.cc
• M remoting/host/setup/me2me_native_messaging_host_unittest.cc
• M remoting/host/win/event_trace_data_unittest.cc

Change size: XL

Delta: 41 files changed, 693 insertions(+), 690 deletions(-)

Branch: refs/heads/main

Submit Requirements:

• Code-Review: +1 by Yuwei Huang

Open in Gerrit

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

Gerrit

Gerrit-MessageType: merged

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: Id46e4ce129baeb27f43d534d4251d6418791a6f9

Gerrit-Change-Number: 7721702

Gerrit-PatchSet: 6

Gerrit-Owner: Joe Downing <joe...@chromium.org>

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

Gerrit-Reviewer: Joe Downing <joe...@chromium.org>

Gerrit-Reviewer: Yuwei Huang <yuw...@chromium.org>

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

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