Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2683453002:
Extended HistoryUITestCase to test titles. (Closed)
|
Created:
3 years, 10 months ago by Eugene But (OOO till 7-30) Modified:
3 years, 10 months ago CC:
chromium-reviews, marq+watch_chromium.org, pkl (ping after 24h if needed), noyau+watch_chromium.org, sdefresne+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionExtended HistoryUITestCase to test titles.
Also added disabled testHistoryUpdateAfterBackNavigation as a test case
for crbug.com/688047 and cleaned up WebViewContainingText matcher usage.
BUG=688047
Review-Url: https://codereview.chromium.org/2683453002
Cr-Commit-Position: refs/heads/master@{#448762}
Committed: https://chromium.googlesource.com/chromium/src/+/9fc98ecff33175b1bc234f9c6c3f42c07285dc1f
Patch Set 1 #Patch Set 2 : Fixed iPad tests. #Patch Set 3 : Added comments #
Total comments: 2
Patch Set 4 : Removed autorelease #Messages
Total messages: 27 (18 generated)
The CQ bit was checked by eugenebut@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.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by eugenebut@chromium.org to run a CQ dry run
eugenebut@chromium.org changed reviewers: + baxley@chromium.org
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2683453002/diff/40001/ios/chrome/browser/ui/h... File ios/chrome/browser/ui/history/history_ui_egtest.mm (right): https://codereview.chromium.org/2683453002/diff/40001/ios/chrome/browser/ui/h... ios/chrome/browser/ui/history/history_ui_egtest.mm:73: descriptionBlock:describe] autorelease], Mike, I tried running this on downstream bots and got ARC-related compilation failure: ../../ios/chrome/browser/ui/history/history_ui_egtest.mm:77:56: error: 'autorelease' is unavailable: not available in automatic reference counting mode descriptionBlock:describe] autorelease], ^ /b/build/slave/ipad10-simulator/build/src/build/ios_files/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator10.2.sdk/usr/include/objc/NSObject.h:38:1: note: 'autorelease' has been explicitly marked unavailable here - (instancetype)autorelease OBJC_ARC_UNAVAILABLE; ^ ../../ios/chrome/browser/ui/history/history_ui_egtest.mm:77:56: error: ARC forbids explicit message send of 'autorelease' descriptionBlock:describe] autorelease], ~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ 2 errors generated. But this file is not ARC. Do you know where can be a problem?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
On 2017/02/07 02:47:34, Eugene But wrote: > https://codereview.chromium.org/2683453002/diff/40001/ios/chrome/browser/ui/h... > File ios/chrome/browser/ui/history/history_ui_egtest.mm (right): > > https://codereview.chromium.org/2683453002/diff/40001/ios/chrome/browser/ui/h... > ios/chrome/browser/ui/history/history_ui_egtest.mm:73: > descriptionBlock:describe] autorelease], > Mike, I tried running this on downstream bots and got ARC-related compilation > failure: > > ../../ios/chrome/browser/ui/history/history_ui_egtest.mm:77:56: error: > 'autorelease' is unavailable: not available in automatic reference counting mode > descriptionBlock:describe] autorelease], > ^ > /b/build/slave/ipad10-simulator/build/src/build/ios_files/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator10.2.sdk/usr/include/objc/NSObject.h:38:1: > note: 'autorelease' has been explicitly marked unavailable here > - (instancetype)autorelease OBJC_ARC_UNAVAILABLE; > ^ > ../../ios/chrome/browser/ui/history/history_ui_egtest.mm:77:56: error: ARC > forbids explicit message send of 'autorelease' > descriptionBlock:describe] autorelease], > ~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ > 2 errors generated. > > > > But this file is not ARC. Do you know where can be a problem? The target //ios/chrome/browser/ui/history:eg_tests that contains history_ui_egtest.mm has been converted yesterday to ARC by https://codereview.chromium.org/2673553003. If you rebase, you'll see that the file now has the ARC header guard and is compiled with ARC.
stkhapugin@chromium.org changed reviewers: + stkhapugin@chromium.org
You'll have to rebase because I've converted this file to ARC, sorry: https://codereview.chromium.org/2673553003 I don't see anything in your change that is incorrect with ARC, so you shouldn't need any specific memory-related changes.
https://codereview.chromium.org/2683453002/diff/40001/ios/chrome/browser/ui/h... File ios/chrome/browser/ui/history/history_ui_egtest.mm (right): https://codereview.chromium.org/2683453002/diff/40001/ios/chrome/browser/ui/h... ios/chrome/browser/ui/history/history_ui_egtest.mm:73: descriptionBlock:describe] autorelease], On 2017/02/07 02:47:34, Eugene But wrote: > Mike, I tried running this on downstream bots and got ARC-related compilation > failure: > > ../../ios/chrome/browser/ui/history/history_ui_egtest.mm:77:56: error: > 'autorelease' is unavailable: not available in automatic reference counting mode > descriptionBlock:describe] autorelease], > ^ > /b/build/slave/ipad10-simulator/build/src/build/ios_files/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator10.2.sdk/usr/include/objc/NSObject.h:38:1: > note: 'autorelease' has been explicitly marked unavailable here > - (instancetype)autorelease OBJC_ARC_UNAVAILABLE; > ^ > ../../ios/chrome/browser/ui/history/history_ui_egtest.mm:77:56: error: ARC > forbids explicit message send of 'autorelease' > descriptionBlock:describe] autorelease], > ~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ > 2 errors generated. > > > > But this file is not ARC. Do you know where can be a problem? > Oops, I haven't spotted this one at first. Just rebase and remove autorelease here (and all other retain/autorelease/release). If you have further questions, please feel free to loop me into the further review.
lgtm
The CQ bit was checked by eugenebut@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.chromium.or...
On 2017/02/07 12:35:15, stkhapugin wrote: > https://codereview.chromium.org/2683453002/diff/40001/ios/chrome/browser/ui/h... > File ios/chrome/browser/ui/history/history_ui_egtest.mm (right): > > https://codereview.chromium.org/2683453002/diff/40001/ios/chrome/browser/ui/h... > ios/chrome/browser/ui/history/history_ui_egtest.mm:73: > descriptionBlock:describe] autorelease], > On 2017/02/07 02:47:34, Eugene But wrote: > > Mike, I tried running this on downstream bots and got ARC-related compilation > > failure: > > > > ../../ios/chrome/browser/ui/history/history_ui_egtest.mm:77:56: error: > > 'autorelease' is unavailable: not available in automatic reference counting > mode > > descriptionBlock:describe] autorelease], > > ^ > > > /b/build/slave/ipad10-simulator/build/src/build/ios_files/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator10.2.sdk/usr/include/objc/NSObject.h:38:1: > > note: 'autorelease' has been explicitly marked unavailable here > > - (instancetype)autorelease OBJC_ARC_UNAVAILABLE; > > ^ > > ../../ios/chrome/browser/ui/history/history_ui_egtest.mm:77:56: error: ARC > > forbids explicit message send of 'autorelease' > > descriptionBlock:describe] autorelease], > > ~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ > > 2 errors generated. > > > > > > > > But this file is not ARC. Do you know where can be a problem? > > > > Oops, I haven't spotted this one at first. Just rebase and remove autorelease > here (and all other retain/autorelease/release). If you have further questions, > please feel free to loop me into the further review. Thanks! Good to know that I'm not crazy :)
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by eugenebut@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from baxley@chromium.org Link to the patchset: https://codereview.chromium.org/2683453002/#ps60001 (title: "Removed autorelease")
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": 60001, "attempt_start_ts": 1486500787228980,
"parent_rev": "443064bb9932a98ad47c1cbbf8bba2493a31f888", "commit_rev":
"9fc98ecff33175b1bc234f9c6c3f42c07285dc1f"}
Message was sent while issue was closed.
Description was changed from ========== Extended HistoryUITestCase to test titles. Also added disabled testHistoryUpdateAfterBackNavigation as a test case for crbug.com/688047 and cleaned up WebViewContainingText matcher usage. BUG=688047 ========== to ========== Extended HistoryUITestCase to test titles. Also added disabled testHistoryUpdateAfterBackNavigation as a test case for crbug.com/688047 and cleaned up WebViewContainingText matcher usage. BUG=688047 Review-Url: https://codereview.chromium.org/2683453002 Cr-Commit-Position: refs/heads/master@{#448762} Committed: https://chromium.googlesource.com/chromium/src/+/9fc98ecff33175b1bc234f9c6c3f... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/chromium/src/+/9fc98ecff33175b1bc234f9c6c3f... |
