Skip to content

android: bump versions of android test dependencies#2565

Merged
Augustyniak merged 5 commits intomainfrom
bump-android-test-dependencies
Sep 27, 2022
Merged

android: bump versions of android test dependencies#2565
Augustyniak merged 5 commits intomainfrom
bump-android-test-dependencies

Conversation

@Augustyniak
Copy link
Contributor

@Augustyniak Augustyniak commented Sep 21, 2022 •
edited
Loading

Description: Update the versions of Android test dependencies.
Risk Level: Low, test impacts test dependencies only
Testing: CI
Docs Changes: N/A
Release Notes: N/A

Signed-off-by: Rafal Augustyniak raugustyniak@lyft.com

@Augustyniak
bump android test dependencies
7e1b93a 
Signed-off-by: Rafal Augustyniak <raugustyniak@lyft.com>
@Augustyniak
fix robolectric bump
16af9d7 
Signed-off-by: Rafal Augustyniak <raugustyniak@lyft.com>
@Augustyniak Augustyniak changed the title bump android test dependencies android: bump versions of android test dependencies Sep 21, 2022
@Augustyniak
Copy link
Contributor Author

Augustyniak commented Sep 21, 2022

/retest

@Augustyniak
Merge branch 'main' into bump-android-test-dependencies
720c2ca 
Signed-off-by: Rafal Augustyniak <raugustyniak@lyft.com>
@Augustyniak
bump robolectric
33b958f 
Signed-off-by: Rafal Augustyniak <raugustyniak@lyft.com>
@Augustyniak
fix formatting
42829b0 
Signed-off-by: Rafal Augustyniak <raugustyniak@lyft.com>
@Augustyniak Augustyniak marked this pull request as ready for review September 22, 2022 19:50
alyssawilk
alyssawilk approved these changes Sep 22, 2022
View reviewed changes
test/java/org/chromium/net/BidirectionalStreamTest.java
checkSpecificErrorCode(NetError.ERR_TIMED_OUT, NetworkException.ERROR_TIMED_OUT, true);
checkSpecificErrorCode(NetError.ERR_ADDRESS_UNREACHABLE,
NetworkException.ERROR_ADDRESS_UNREACHABLE, false);
// checkSpecificErrorCode(NetError.ERR_NAME_NOT_RESOLVED,
Copy link
Contributor

@alyssawilk alyssawilk Sep 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RyanTheOptimist I'm good commenting these out if you are. I hate to have upgrades blocked on it.

Copy link
Contributor

@RyanTheOptimist RyanTheOptimist Sep 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this sounds reasonable, but we should probably file an issue about this so we don't lose track of it. Maybe @colibie has thoughts on how to resolve the missing symbol issues.

Copy link
Contributor

@colibie colibie Sep 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out the previous impl was using the symbols from the cronet in the android platform. Will fix this with PR2568

Copy link
Contributor Author

@Augustyniak Augustyniak Sep 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@colibie thank you for the context. I am going to wait with my PR until yours is merged then.

Copy link
Contributor

@colibie colibie Sep 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can merge yours, I'll update mine. Still making some changes on it anyway.

Copy link
Contributor Author

@Augustyniak Augustyniak Sep 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without your changes I need to leave one test commented out (https://github.com/envoyproxy/envoy-mobile/pull/2565/files#diff-8485610ff0b92707ee93e0eea000f53290572a1a726520b879a1d15580f0e9bfR1545) as we are missing NetError symbols. I guess I can merge my PR in here and follow up on the added TODO once your PR is merged.

@Augustyniak Augustyniak mentioned this pull request Sep 23, 2022
Closed
@Augustyniak Augustyniak merged commit f8b537a into main Sep 27, 2022
@Augustyniak Augustyniak deleted the bump-android-test-dependencies branch September 27, 2022 13:57
jpsim added a commit that referenced this pull request Sep 27, 2022
@jpsim
Merge remote-tracking branch 'origin/main' into bazel-update-rules_xc…
718c1a6 
…odeproj-to-0.8.0

* origin/main:
  bazel: update rules_apple and rules_swift (#2571)
  android: bump versions of android test dependencies (#2565)

Signed-off-by: JP Simard <jp@jpsim.com>
@colibie colibie mentioned this pull request Oct 10, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RyanTheOptimist RyanTheOptimist RyanTheOptimist left review comments

+2 more reviewers

@colibie colibie colibie left review comments

@alyssawilk alyssawilk alyssawilk approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@alyssawilk alyssawilk

@RyanTheOptimist RyanTheOptimist

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Augustyniak @alyssawilk @RyanTheOptimist @colibie