Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2871173003:
Revert of Disable BottomSheetTest on pre-Lollipop MR1 (Closed)
DescriptionRevert of Disable BottomSheetTest on pre-Lollipop MR1 (patchset #1 id:1 of https://codereview.chromium.org/2781093004/ )
Reason for revert:
android.support.v7.widget.RecyclerView is a support library class. It can be used regardless of the Android version.
Original issue's description:
> Disable BottomSheetTest on pre-Lollipop MR1
>
> BottomSheetTests have a hard dependency on RecyclerView,
> which is a class that was added in SDK level 22 (Lollipop
> MR 1).
> This change disables BottomSheetTests on all API levels
> which do not have RecyclerView.
>
> BUG=None
>
> Review-Url: https://codereview.chromium.org/2781093004
> Cr-Commit-Position: refs/heads/master@{#460554}
> Committed: https://chromium.googlesource.com/chromium/src/+/7038439b2c151c9dd0fcd1de0330f2785d11d09a
TBR=nyquist@chromium.org,kraush@amazon.com
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=None
Review-Url: https://codereview.chromium.org/2871173003
Cr-Commit-Position: refs/heads/master@{#470976}
Committed: https://chromium.googlesource.com/chromium/src/+/1ddae8b5060f149a7819414f416713840c640ba7
Patch Set 1 #
Messages
Total messages: 22 (9 generated)
The CQ bit was checked by twellington@chromium.org
Created Revert of Disable BottomSheetTest on pre-Lollipop MR1
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2017/05/10 21:15:29, commit-bot: I haz the power wrote: > CQ is trying da patch. > > Follow status at: > https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or... Do any of your bots run on KitKat phones? I'm pretty sure we were seeing ClassNotFoundExceptions on KitKat phones with this test. Building a master-workspace right now to make sure I'm not making things up here.
On 2017/05/10 21:24:56, kraush wrote: > On 2017/05/10 21:15:29, commit-bot: I haz the power wrote: > > CQ is trying da patch. > > > > Follow status at: > > > https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or... > > Do any of your bots run on KitKat phones? > I'm pretty sure we were seeing ClassNotFoundExceptions on KitKat phones with > this test. > Building a master-workspace right now to make sure I'm not making things up > here. Yes, we have bots that run KitKat. The support library is designed for backward compatibility: https://developer.android.com/topic/libraries/support-library/index.html. A quick code search will show numerous other tests that use android.support.v7.widget.RecyclerView. Lint throws errors if Android classes or methods are used that are not available on our minimum supported Android API level (16).
On 2017/05/10 21:35:55, Theresa wrote: > > Yes, we have bots that run KitKat. > > The support library is designed for backward compatibility: > https://developer.android.com/topic/libraries/support-library/index.html. A > quick code search will show numerous other tests that use > android.support.v7.widget.RecyclerView. > > Lint throws errors if Android classes or methods are used that are not available > on our minimum supported Android API level (16). Gotcha, thanks Theresa, makes sense! I'm wondering if proguard could be tearing them out then? I also just noticed that all tests that use RecyclerView either a) Have this annotation: @RunWith(BaseJUnit4ClassRunner.class) b) Are set to run on only phones. --> Just to confirm, are any of your KitKat testers running on phones?
On 2017/05/10 21:43:44, kraush wrote: > On 2017/05/10 21:35:55, Theresa wrote: > > > > Yes, we have bots that run KitKat. > > > > The support library is designed for backward compatibility: > > https://developer.android.com/topic/libraries/support-library/index.html. A > > quick code search will show numerous other tests that use > > android.support.v7.widget.RecyclerView. > > > > Lint throws errors if Android classes or methods are used that are not > available > > on our minimum supported Android API level (16). > > Gotcha, thanks Theresa, makes sense! > I'm wondering if proguard could be tearing them out then? I don't think so. > I also just noticed that all tests that use RecyclerView either > a) Have this annotation: @RunWith(BaseJUnit4ClassRunner.class) This annotation just added today: https://codereview.chromium.org/2853573002 > b) Are set to run on only phones. > --> Just to confirm, are any of your KitKat testers running on phones? Yes.
The CQ bit was unchecked by twellington@chromium.org
This CL is reliant on another that fixes one failing test and disables another: https://codereview.chromium.org/2875813002/ One of these test would have been caught by the CQ had the annotation for L MR1 not been added.
The CQ bit was checked by twellington@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.
lgtm
lgtm Sorry for the delay! :) Just finished two test runs each on a KitKat phone, and a Nexus 5X running N. Everything working fine on the KitKat device now - whatever was failing before must've been fixed somewhere down the line! :) On a sidenote, I got a flake of BottomSheetContentControllerTest#testSelectContent on the Nougat device, but that's obviously unrelated.
On 2017/05/11 16:26:07, kraush wrote: > lgtm > > Sorry for the delay! :) > Just finished two test runs each on a KitKat phone, and a Nexus 5X running N. > > Everything working fine on the KitKat device now - whatever was failing before > must've been fixed somewhere down the line! :) > > On a sidenote, I got a flake of > BottomSheetContentControllerTest#testSelectContent on the Nougat device, but > that's obviously unrelated. BottomSheetContentControllerTest#testSelectContent was failing. I fixed it in a patch that landed ~12 hours ago (https://codereview.chromium.org/2875813002/), so you may try syncing again.
The CQ bit was checked by twellington@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": 1, "attempt_start_ts": 1494520412302680, "parent_rev":
"b25681c4b38efe3f3a7a9bff034f9e83170b1d00", "commit_rev":
"1ddae8b5060f149a7819414f416713840c640ba7"}
Message was sent while issue was closed.
Description was changed from ========== Revert of Disable BottomSheetTest on pre-Lollipop MR1 (patchset #1 id:1 of https://codereview.chromium.org/2781093004/ ) Reason for revert: android.support.v7.widget.RecyclerView is a support library class. It can be used regardless of the Android version. Original issue's description: > Disable BottomSheetTest on pre-Lollipop MR1 > > BottomSheetTests have a hard dependency on RecyclerView, > which is a class that was added in SDK level 22 (Lollipop > MR 1). > This change disables BottomSheetTests on all API levels > which do not have RecyclerView. > > BUG=None > > Review-Url: https://codereview.chromium.org/2781093004 > Cr-Commit-Position: refs/heads/master@{#460554} > Committed: https://chromium.googlesource.com/chromium/src/+/7038439b2c151c9dd0fcd1de0330... TBR=nyquist@chromium.org,kraush@amazon.com # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=None ========== to ========== Revert of Disable BottomSheetTest on pre-Lollipop MR1 (patchset #1 id:1 of https://codereview.chromium.org/2781093004/ ) Reason for revert: android.support.v7.widget.RecyclerView is a support library class. It can be used regardless of the Android version. Original issue's description: > Disable BottomSheetTest on pre-Lollipop MR1 > > BottomSheetTests have a hard dependency on RecyclerView, > which is a class that was added in SDK level 22 (Lollipop > MR 1). > This change disables BottomSheetTests on all API levels > which do not have RecyclerView. > > BUG=None > > Review-Url: https://codereview.chromium.org/2781093004 > Cr-Commit-Position: refs/heads/master@{#460554} > Committed: https://chromium.googlesource.com/chromium/src/+/7038439b2c151c9dd0fcd1de0330... TBR=nyquist@chromium.org,kraush@amazon.com # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=None Review-Url: https://codereview.chromium.org/2871173003 Cr-Commit-Position: refs/heads/master@{#470976} Committed: https://chromium.googlesource.com/chromium/src/+/1ddae8b5060f149a7819414f4167... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/1ddae8b5060f149a7819414f4167...
Message was sent while issue was closed.
On 2017/05/11 16:33:28, Theresa wrote: > > BottomSheetContentControllerTest#testSelectContent was failing. I fixed it in a > patch that landed ~12 hours ago (https://codereview.chromium.org/2875813002/), > so you may try syncing again. I had actually fetched about an hour before posting (because I had seen your previous comment on the issue) --> So I'm fairly certain I have that change. Just to be clear though, it was a flake only. (Worked on the second attempt) |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
