Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2838073002:
[shared_page_state] Fix unit test (Closed)
Description[shared_page_state] Fix unit test
BUG=chromium:714803
TEST=run unittest on CrOS
Signed-off-by: Chung-yih Wang <cywang@chromium.org>;
Review-Url: https://codereview.chromium.org/2838073002
Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/778079b344c87eb05b021cffbdfb08eb3d92641c
Patch Set 1 #
Total comments: 2
Patch Set 2 : [shared_page_state] Fix unit test #Patch Set 3 : [shared_page_state] Fix unit test testPageStatesUserAgentType #Messages
Total messages: 28 (13 generated)
cywang@chromium.org changed reviewers: + michaelpg@chromium.org, warx@chromium.org
cywang@chromium.org changed reviewers: + achuith@chromium.org, nednguyen@chromium.org
lgtm
The CQ bit was checked by achuith@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Catapult Mac Tryserver on master.tryserver.client.catapult (JOB_FAILED, https://build.chromium.org/p/tryserver.client.catapult/builders/Catapult%20Ma...)
The CQ bit was checked by cywang@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Catapult Mac Tryserver on master.tryserver.client.catapult (JOB_FAILED, https://build.chromium.org/p/tryserver.client.catapult/builders/Catapult%20Ma...)
nednguyen@google.com changed reviewers: + nednguyen@google.com
https://codereview.chromium.org/2838073002/diff/1/telemetry/telemetry/page/sh... File telemetry/telemetry/page/shared_page_state_unittest.py (right): https://codereview.chromium.org/2838073002/diff/1/telemetry/telemetry/page/sh... telemetry/telemetry/page/shared_page_state_unittest.py:86: if platform_module.GetHostPlatform().GetOSName() == 'chromeos': can you just change this flow to: if platform_module.GetHostPlatform().GetOSName() == 'chromeos': self.assertUserAgentSetCorrectly(shared_page_state.SharedDesktopPageState, 'chromeos') else: self.assert...('desktop')
Thanks! https://codereview.chromium.org/2838073002/diff/1/telemetry/telemetry/page/sh... File telemetry/telemetry/page/shared_page_state_unittest.py (right): https://codereview.chromium.org/2838073002/diff/1/telemetry/telemetry/page/sh... telemetry/telemetry/page/shared_page_state_unittest.py:86: if platform_module.GetHostPlatform().GetOSName() == 'chromeos': On 2017/04/25 12:54:44, nednguyen wrote: > can you just change this flow to: > if platform_module.GetHostPlatform().GetOSName() == 'chromeos': > > self.assertUserAgentSetCorrectly(shared_page_state.SharedDesktopPageState, > 'chromeos') > else: > self.assert...('desktop') Done.
On 2017/04/25 13:14:18, cywang wrote: > Thanks! > > https://codereview.chromium.org/2838073002/diff/1/telemetry/telemetry/page/sh... > File telemetry/telemetry/page/shared_page_state_unittest.py (right): > > https://codereview.chromium.org/2838073002/diff/1/telemetry/telemetry/page/sh... > telemetry/telemetry/page/shared_page_state_unittest.py:86: if > platform_module.GetHostPlatform().GetOSName() == 'chromeos': > On 2017/04/25 12:54:44, nednguyen wrote: > > can you just change this flow to: > > if platform_module.GetHostPlatform().GetOSName() == 'chromeos': > > > > self.assertUserAgentSetCorrectly(shared_page_state.SharedDesktopPageState, > > 'chromeos') > > else: > > self.assert...('desktop') > > Done. lgtm but please update the title & description to be more specific before landing. Fix which unittest? What is the fix about? etc...
The CQ bit was checked by cywang@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from achuith@chromium.org, nednguyen@google.com Link to the patchset: https://codereview.chromium.org/2838073002/#ps40001 (title: "[shared_page_state] Fix unit test testPageStatesUserAgentType")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Catapult Mac Tryserver on master.tryserver.client.catapult (JOB_FAILED, https://build.chromium.org/p/tryserver.client.catapult/builders/Catapult%20Ma...)
The CQ bit was checked by cywang@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1493352020113610,
"parent_rev": "9c9ac13a2b028b9aa24257dc9e669eda29a301ea", "commit_rev":
"778079b344c87eb05b021cffbdfb08eb3d92641c"}
Message was sent while issue was closed.
Description was changed from ========== [shared_page_state] Fix unit test BUG=chromium:714803 TEST=run unittest on CrOS Signed-off-by: Chung-yih Wang <cywang@chromium.org> ========== to ========== [shared_page_state] Fix unit test BUG=chromium:714803 TEST=run unittest on CrOS Signed-off-by: Chung-yih Wang <cywang@chromium.org> Review-Url: https://codereview.chromium.org/2838073002 Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapu... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/external/github.com/catapult-project/catapu... |
