Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NT-1735: Target API 30 🚀 #1240

Merged
merged 20 commits into from
May 12, 2021
Merged

NT-1735: Target API 30 🚀 #1240

merged 20 commits into from
May 12, 2021

Conversation

leighdouglas
Copy link
Contributor

@leighdouglas leighdouglas commented May 10, 2021

📲 What

Upgrading to Android API 30 requires a few updates for our codebase to be compatible. I have added a queries element to the Android manifest file for the new package visibility requirements in API 30. Also updated tests to reflect new formatting changes.

  • Added a new queries element to android manifest.
  • Updated the compileSdkVersion and targetSdkVersion to 30
  • Upgraded robolectric dependency to 4.4
  • Retrieving a project object from an intent now checks for nullability
  • Set KSCurrency with a mock current config to the mock environment object used for testing view models to pass tests failing on the external release variant
  • Disabled pausing and unpausing the shadow looper in the ObserveForUITransformerTest suite (more on that below)
  • Amended the jacoco config struct to exclude coverage for internal jdk classes -> this is necessary since we are using JDK 1.8 (more on that below)
  • Updated the codecov config struct to be informational and not pass/fail PRs
  • Updated failing tests with new formatting to Date and Number strings:
Dec 17, 2015 6:35 PM -> Dec 17, 2015, 6:35 PM
17 déc. 2015 18:35:05 -> 17 déc. 2015 à 18:35:05
100 $ -> now has a non-breaking space

🛠 How

More context on ObserveForUITransformerTest -> Robolectric 4.4 was a massive update that included changes to their shadow looper. These changes are causing mysterious test failures during the tearDown() that happens after the test is run, specifically it is throwing an java.lang.UnsupportedOperationException: main looper cannot be unpaused exception. After exhausting a few solutions from their documentation (they have a whole article about it here-> http://robolectric.org/blog/2019/06/04/paused-looper/), and since all of this code is about 5 years old, @jgsamudio and I decided to remove these lines since the tests are passing without them. Perhaps robolectric is not performing these operations for us under the hood.

For the jacoco config to exclude generating coverage for jvm internal classes, this is a known issue and related to our target jvm being 1.8. See -> gradle/gradle#5184

👀 See

No UI changes

📋 QA

Since these are all internal changes and not user facing, I would test this by opening the app and making sure nothing is crashing.

Story 📖

[SPIKE] Fix Test Environment for Android 11 Migration
[Spike]Android 11: Target API 30

@codecov
Copy link

codecov bot commented May 11, 2021

Codecov Report

Merging #1240 (7ebd2f7) into master (acfa137) will decrease coverage by 0.15%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1240      +/-   ##
============================================
- Coverage     74.85%   74.69%   -0.16%     
+ Complexity      738      734       -4     
============================================
  Files           221      221              
  Lines          6645     6656      +11     
  Branches        402      402              
============================================
- Hits           4974     4972       -2     
- Misses         1538     1551      +13     
  Partials        133      133              
Impacted Files Coverage Δ Complexity Δ
.../main/java/com/kickstarter/libs/CurrentConfig.java 68.75% <0.00%> (-15.63%) 2.00% <0.00%> (-4.00%)
...c/main/java/com/kickstarter/libs/ApiPaginator.java 80.45% <0.00%> (-1.15%) 16.00% <0.00%> (ø%)
...va/com/kickstarter/viewmodels/UpdateViewModel.java 95.23% <0.00%> (-1.15%) 0.00% <0.00%> (ø%)
.../com/kickstarter/viewmodels/MessagesViewModel.java 96.05% <0.00%> (-0.81%) 0.00% <0.00%> (ø%)
...va/com/kickstarter/viewmodels/ThanksViewModel.java 98.18% <0.00%> (ø) 0.00% <0.00%> (ø%)
...ickstarter/ui/adapters/DiscoveryDrawerAdapter.java 0.00% <0.00%> (ø) 0.00% <0.00%> (ø%)
.../com/kickstarter/viewmodels/CommentsViewModel.java 96.53% <0.00%> (+0.02%) 0.00% <0.00%> (ø%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update acfa137...7ebd2f7. Read the comment docs.

@@ -49,7 +51,13 @@ abstract class KSRobolectricTestCase : TestCase() {
.applicationModule(TestApplicationModule(application()))
.build()

val config = ConfigFactory.config().toBuilder()
Copy link
Contributor

Choose a reason for hiding this comment

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

nice!

@@ -23,13 +23,13 @@
@Before
public void setUp() {
RxAndroidPlugins.getInstance().reset();
ShadowLooper.pauseMainLooper();
//ShadowLooper.pauseLooper(Looper.myLooper());
Copy link
Contributor

Choose a reason for hiding this comment

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

delete this two lines please

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 👍🏽

Copy link
Contributor

@Arkariang Arkariang left a comment

Choose a reason for hiding this comment

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

Great work here @leighdouglas!

@@ -94,6 +96,7 @@ class BackingFragmentViewModelTest : KSRobolectricTestCase() {
this.vm.outputs.projectDataAndAddOns().subscribe(this.listAddOns)
this.vm.outputs.bonusSupport().map { it.toString() }.subscribe(this.bonusAmount)
this.vm.outputs.deliveryDisclaimerSectionIsGone().subscribe(this.disclaimerSectionIsGone)
Shadows.shadowOf(Looper.getMainLooper()).idle()
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these Shadows.shadowOf(Looper.getMainLooper()).idle() are no longer necessary as you found the root cause if blocking problem

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay i can remove them and see if it passes CI

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, and everything passed CI! 🎉 🚀

@leighdouglas leighdouglas marked this pull request as ready for review May 11, 2021 21:14
@leighdouglas leighdouglas changed the title Feature/adopt api 30 NT-1735: Fix Test Environment for Android 11 Migration May 11, 2021
@leighdouglas leighdouglas changed the title NT-1735: Fix Test Environment for Android 11 Migration NT-1735: Target API 30 🚀 May 11, 2021
@leighdouglas leighdouglas merged commit 066d87b into master May 12, 2021
@leighdouglas leighdouglas deleted the feature/adopt-api-30 branch May 12, 2021 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants