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

[FC] Removes mavericks from all viewmodels and tests #8155

Conversation

carlosmuvi-stripe
Copy link
Collaborator

@carlosmuvi-stripe carlosmuvi-stripe commented Mar 26, 2024

Summary

  • Moves remaining ViewModels and all tests out of Mavericks.
  • Removes mavericks compose and mavericks test dependencies

Motivation

Testing

  • Added tests
  • Modified tests
  • Manually verified

Screenshots

Before After
before screenshot after screenshot

Changelog

@carlosmuvi-stripe carlosmuvi-stripe changed the title carlosmuvi/remove-mavericks-part-3 [FC] Removes mavericks from all viewmodels and tests Mar 26, 2024
Copy link
Contributor

github-actions bot commented Mar 26, 2024

Diffuse output:

OLD: identity-example-release-base.apk (signature: V1, V2)
NEW: identity-example-release-pr.apk (signature: V1, V2)

          │          compressed          │         uncompressed         
          ├───────────┬───────────┬──────┼───────────┬───────────┬──────
 APK      │ old       │ new       │ diff │ old       │ new       │ diff 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
      dex │     2 MiB │     2 MiB │  0 B │   4.2 MiB │   4.2 MiB │  0 B 
     arsc │   1.5 MiB │   1.5 MiB │  0 B │   1.5 MiB │   1.5 MiB │  0 B 
 manifest │   2.3 KiB │   2.3 KiB │  0 B │   8.1 KiB │   8.1 KiB │  0 B 
      res │ 363.2 KiB │ 363.2 KiB │  0 B │ 490.6 KiB │ 490.6 KiB │  0 B 
   native │   7.3 MiB │   7.3 MiB │  0 B │  18.4 MiB │  18.4 MiB │  0 B 
    asset │   1.6 MiB │   1.6 MiB │  0 B │   1.6 MiB │   1.6 MiB │  0 B 
    other │ 116.1 KiB │ 116.1 KiB │  0 B │ 273.1 KiB │ 273.1 KiB │  0 B 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
    total │  12.8 MiB │  12.8 MiB │  0 B │  26.4 MiB │  26.4 MiB │  0 B 

 DEX     │ old   │ new   │ diff      
─────────┼───────┼───────┼───────────
   files │     1 │     1 │ 0         
 strings │ 21414 │ 21414 │ 0 (+0 -0) 
   types │  6641 │  6641 │ 0 (+0 -0) 
 classes │  5408 │  5408 │ 0 (+0 -0) 
 methods │ 31079 │ 31079 │ 0 (+0 -0) 
  fields │ 18096 │ 18096 │ 0 (+0 -0) 

 ARSC    │ old  │ new  │ diff 
─────────┼──────┼──────┼──────
 configs │  262 │  262 │  0   
 entries │ 5514 │ 5514 │  0

Base automatically changed from carlosmuvi/remove-mavericks-part-2 to carlosmuvi/i/remove-mavericks March 26, 2024 16:31
…nto carlosmuvi/remove-mavericks-part-3

# Conflicts:
#	financial-connections/src/test/java/com/stripe/android/financialconnections/features/accountpicker/AccountPickerViewModelTest.kt
#	financial-connections/src/test/java/com/stripe/android/financialconnections/features/partnerauth/PartnerAuthViewModelTest.kt
@carlosmuvi-stripe carlosmuvi-stripe marked this pull request as ready for review March 26, 2024 16:44
@carlosmuvi-stripe carlosmuvi-stripe requested review from a team as code owners March 26, 2024 16:44
@carlosmuvi-stripe carlosmuvi-stripe requested review from jaynewstrom-stripe and tillh-stripe and removed request for a team and jaynewstrom-stripe March 26, 2024 16:44
Copy link
Collaborator

@tillh-stripe tillh-stripe left a comment

Choose a reason for hiding this comment

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

Love that we’re getting rid of this ❤️

@@ -994,15 +994,6 @@
| | | \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.8.22 -> 1.9.10 (*)
| | +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:{require 1.6.4; reject _} -> 1.7.3 (*)
| | \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.8.22 -> 1.9.10 (*)
| +--- com.airbnb.android:mavericks-compose:3.0.9
Copy link
Collaborator

Choose a reason for hiding this comment

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

🔥

Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we rename this file or move the extensions?

@@ -42,7 +43,7 @@ import org.mockito.kotlin.whenever
class LinkAccountPickerViewModelTest {

@get:Rule
val mavericksTestRule = MavericksTestRule()
val testRule = CoroutineTestRule(UnconfinedTestDispatcher())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit!

Suggested change
val testRule = CoroutineTestRule(UnconfinedTestDispatcher())
val testRule = CoroutineTestRule()

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@@ -109,7 +109,7 @@ class LinkStepUpVerificationViewModelTest {

val viewModel = buildViewModel()

assertThat(viewModel.awaitState().payload).isInstanceOf(Loading::class.java)
assertThat(viewModel.stateFlow.value.payload).isInstanceOf(Loading::class.java)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we test this with Turbine to be more explicit about when we expect a new state and when we don’t?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

(Added an example asserting no events are emitted after consumer lookup results in no user found)

@get:Rule
val mavericksTestRule = MavericksTestRule(testDispatcher = testDispatcher)
val testRule = CoroutineTestRule(UnconfinedTestDispatcher())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit!

Suggested change
val testRule = CoroutineTestRule(UnconfinedTestDispatcher())
val testRule = CoroutineTestRule()

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@carlosmuvi-stripe carlosmuvi-stripe merged commit 1f0f397 into carlosmuvi/i/remove-mavericks Mar 27, 2024
12 of 13 checks passed
@carlosmuvi-stripe carlosmuvi-stripe deleted the carlosmuvi/remove-mavericks-part-3 branch March 27, 2024 00:41
carlosmuvi-stripe added a commit that referenced this pull request Mar 27, 2024
* [FC] Moves Activities and some Panes out of Mavericks (#8125)

* Removes mackericks references on Consent screen.

* Updates files.

* Uses viewmodel factory builder.

* Updates functions.

* Updates execute.

* Updates compose util.

* Adds missing side effects.

* Simplifies code.

* Renames viewmodel.

* Updates async.

* Removes mavericks from institution picker.

* Reverts rename.

* Updates baseline.

* Adds setState and persists state.

* Renames result error to fail.

* Migrates Initial activity out of mavericks.

* Removes persist state.

* Updates tests.

* Regenerates API.

* filterNotNull.

* Nits.

* Updates async.

* use suspend block.

* Update financial-connections/src/main/java/com/stripe/android/financialconnections/core/FinancialConnectionsViewModel.kt

Co-authored-by: Till Hellmund <tillh@stripe.com>

* Tries onAsync.

* Moves activity to stripe ui core.

* PR feedback.

* Regenerates deps.

---------

Co-authored-by: Till Hellmund <tillh@stripe.com>

* [FC] Removes mavericks from repositories and more panes. (#8154)

* Migrates more screens out of mavs.

* Migrates partner auth.

* Removes active auth session field.

* Updates tests.

* Updates attach payment viewmodel.

* PR feedback.

* Api dump.

* [FC] Removes mavericks from all viewmodels and tests (#8155)

* Migrates more screens out of mavs.

* Migrates partner auth.

* Removes active auth session field.

* Updates tests.

* Updates attach payment viewmodel.

* PR feedback.

* Api dump.

* Migrates missing viewmodels.

* Updates dependencies.

* PR feedback.

* Merge master.

* [FC] Removes mavericks dependency (#8160)

* Migrates more screens out of mavs.

* Migrates partner auth.

* Removes active auth session field.

* Updates tests.

* Updates attach payment viewmodel.

* PR feedback.

* Api dump.

* Migrates missing viewmodels.

* Updates dependencies.

* Removes mavericks dependency.

* Merge with integration.

* Uses collect.

* Updates dependencies

* Updates Changelog.

* Update CHANGELOG.md

Co-authored-by: Till Hellmund <tillh@stripe.com>

---------

Co-authored-by: Till Hellmund <tillh@stripe.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants