Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2891593003:
Add DesktopRectTest for UnionWith() function (Closed)
DescriptionAdd DesktopRectTest for UnionWith() function
DesktopRect::UnionWith() function has been added by change
https://codereview.webrtc.org/2845213002. This change adds test cases to cover
the newly added logic. More specifically, union between an empty rectangle and a
non-empty one or two empty rectangles.
BUG=webrtc:7541
Review-Url: https://codereview.webrtc.org/2891593003
Cr-Commit-Position: refs/heads/master@{#18201}
Committed: https://chromium.googlesource.com/external/webrtc/+/8eef7aa7cf165401fe4083b02334890097e5b003
Patch Set 1 #
Total comments: 4
Patch Set 2 : Resolve review comments #
Messages
Total messages: 28 (22 generated)
The CQ bit was checked by zijiehe@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was checked by zijiehe@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
Patchset #2 (id:20001) has been deleted
The CQ bit was checked by zijiehe@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
Patchset #1 (id:1) has been deleted
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Add DesktopRectTest for UnionWith() function DesktopRect::UnionWith() function has been added by change https://codereview.webrtc.org/2845213002. This change adds test cases to cover the newly added logic. More specifically, union between an empty rectangle and a non-empty one or two empty rectangles. BUG=webrtc:7541 ========== to ========== Add DesktopRectTest for UnionWith() function DesktopRect::UnionWith() function has been added by change https://codereview.webrtc.org/2845213002. This change adds test cases to cover the newly added logic. More specifically, union between an empty rectangle and a non-empty one or two empty rectangles. BUG=webrtc:7541 ==========
zijiehe@chromium.org changed reviewers: + sergeyu@chromium.org, wez@chromium.org
sergeyu@google.com changed reviewers: + sergeyu@google.com
lgtm https://codereview.webrtc.org/2891593003/diff/40001/webrtc/modules/desktop_ca... File webrtc/modules/desktop_capture/desktop_geometry_unittest.cc (right): https://codereview.webrtc.org/2891593003/diff/40001/webrtc/modules/desktop_ca... webrtc/modules/desktop_capture/desktop_geometry_unittest.cc:20: ASSERT_TRUE(rect.equals(DesktopRect::MakeXYWH(-2, -2, 4, 4))); nit: this test would be easier to read if it used MakeLTRB instead of MakeXYWH https://codereview.webrtc.org/2891593003/diff/40001/webrtc/modules/desktop_ca... webrtc/modules/desktop_capture/desktop_geometry_unittest.cc:54: ASSERT_TRUE(rect.equals(DesktopRect())); rect.is_empty()?
sergeyu@chromium.org changed reviewers: - wez@chromium.org
lgtm from the right account
The CQ bit was checked by zijiehe@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.webrtc.org/2891593003/diff/40001/webrtc/modules/desktop_ca... File webrtc/modules/desktop_capture/desktop_geometry_unittest.cc (right): https://codereview.webrtc.org/2891593003/diff/40001/webrtc/modules/desktop_ca... webrtc/modules/desktop_capture/desktop_geometry_unittest.cc:20: ASSERT_TRUE(rect.equals(DesktopRect::MakeXYWH(-2, -2, 4, 4))); On 2017/05/18 17:37:53, Do not use (sergeyu) wrote: > nit: this test would be easier to read if it used MakeLTRB instead of MakeXYWH Done. https://codereview.webrtc.org/2891593003/diff/40001/webrtc/modules/desktop_ca... webrtc/modules/desktop_capture/desktop_geometry_unittest.cc:54: ASSERT_TRUE(rect.equals(DesktopRect())); On 2017/05/18 17:37:53, Do not use (sergeyu) wrote: > rect.is_empty()? Done.
The CQ bit was checked by zijiehe@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sergeyu@chromium.org, sergeyu@google.com Link to the patchset: https://codereview.webrtc.org/2891593003/#ps60001 (title: "Resolve review comments")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
CQ is committing da patch.
Bot data: {"patchset_id": 60001, "attempt_start_ts": 1495135453934990,
"parent_rev": "119c40784872c2d8ac78469bfa39044b9c4a5502", "commit_rev":
"8eef7aa7cf165401fe4083b02334890097e5b003"}
Message was sent while issue was closed.
Description was changed from ========== Add DesktopRectTest for UnionWith() function DesktopRect::UnionWith() function has been added by change https://codereview.webrtc.org/2845213002. This change adds test cases to cover the newly added logic. More specifically, union between an empty rectangle and a non-empty one or two empty rectangles. BUG=webrtc:7541 ========== to ========== Add DesktopRectTest for UnionWith() function DesktopRect::UnionWith() function has been added by change https://codereview.webrtc.org/2845213002. This change adds test cases to cover the newly added logic. More specifically, union between an empty rectangle and a non-empty one or two empty rectangles. BUG=webrtc:7541 Review-Url: https://codereview.webrtc.org/2891593003 Cr-Commit-Position: refs/heads/master@{#18201} Committed: https://chromium.googlesource.com/external/webrtc/+/8eef7aa7cf165401fe4083b02... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:60001) as https://chromium.googlesource.com/external/webrtc/+/8eef7aa7cf165401fe4083b02... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
