Skip to content

Remove some of the unused dependencies for tests in each module#548

Merged
tonidero merged 2 commits into
mainfrom
remove-unused-dependencies
Jun 17, 2022
Merged

Remove some of the unused dependencies for tests in each module#548
tonidero merged 2 commits into
mainfrom
remove-unused-dependencies

Conversation

@tonidero

@tonidero tonidero commented Jun 16, 2022

Copy link
Copy Markdown
Contributor

Description

We are adding a bunch of dependencies that are unused on each module. This PR removes some of the easier to remove dependencies. I also removed all the dependencies from library.gradle since those are imported on every module. I feel that dependencies belong to each module but lmk if you think otherwise.

@tonidero tonidero left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is the result of a first pass. It's possible we can remove more dependencies, but didn't want to spend much more time on this.

Comment thread common/build.gradle
testImplementation 'androidx.test.ext:junit:1.1.1'
testImplementation 'org.robolectric:robolectric:4.3'
testImplementation 'com.squareup.okhttp3:mockwebserver:4.2.0'
testImplementation 'org.mockito:mockito-core:3.0.0'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Mockito seems to be unused so I removed it on all the modules

androidTestImplementation 'androidx.test:rules:1.3.0'
androidTestImplementation 'androidx.test:core-ktx:1.3.0'
androidTestImplementation 'androidx.test.ext:junit-ktx:1.1.2'
androidTestImplementation 'org.assertj:assertj-core:3.13.2'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We don't have instrumentation tests on this module, so I removed all of these. Did the same for some of the other modules. Only module with instrumentation tests is the integration-tests module

implementation "org.jetbrains.kotlin:kotlin-stdlib:$kotlinVersion"
implementation 'androidx.core:core-ktx:1.3.1'
implementation 'androidx.appcompat:appcompat:1.2.0'
testImplementation 'junit:junit:4.12'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

junit4 should already be added by some of the other dependencies (I believe robolectric adds it)

Comment thread library.gradle
testImplementation 'androidx.test:rules:1.4.0'
testImplementation 'androidx.test.ext:junit:1.1.3'
testImplementation 'org.robolectric:robolectric:4.6.1'
testImplementation 'com.squareup.okhttp3:mockwebserver:4.2.0'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

mockwebserver is only used on the common module, so I just left it on that module.

Comment thread library.gradle
}
}

dependencies {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I removed the dependencies block from the library.gradle since these are added to every module, even those that don't need it. I think it's fine to have some of the other config in this gradle file (like flavors, java version,...). However, I feel that dependencies belong on each module. Lmk if there are any concerns

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I agree with you 😄

Comment thread library.gradle
testImplementation 'androidx.test:runner:1.4.0'
testImplementation 'androidx.test:rules:1.4.0'
testImplementation 'androidx.test.ext:junit:1.1.3'
testImplementation 'org.robolectric:robolectric:4.6.1'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Note that the versions on the library.gradle file were higher in general than the ones on each module. I believe when there is a conflict of versions, gradle will pick the one with the highest version. So I used the higher version libraries for all the modules. These are all test dependencies so it shouldn't affect users in any case

@tonidero tonidero changed the title Remove some of the unused dependencies on tests on our modules Remove some of the unused dependencies for tests in our modules Jun 16, 2022
@tonidero tonidero changed the title Remove some of the unused dependencies for tests in our modules Remove some of the unused dependencies for tests in each module Jun 16, 2022
@tonidero tonidero requested a review from a team June 16, 2022 14:46
@tonidero tonidero marked this pull request as ready for review June 16, 2022 14:46

@NachoSoto NachoSoto left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice clean up!

Comment thread build.gradle
Comment on lines +9 to +13
ext.testLibrariesVersion = "1.4.0"
ext.testJUnitVersion = "1.1.3"
ext.robolectricVersion = "4.6.1"
ext.mockkVersion = "1.10.0"
ext.assertJVersion = "3.13.2"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

😻

@vegaro vegaro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I love this PR

Comment thread library.gradle
}
}

dependencies {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I agree with you 😄

@tonidero tonidero merged commit a2fb6b5 into main Jun 17, 2022
@tonidero tonidero deleted the remove-unused-dependencies branch June 17, 2022 14:18
@tonidero tonidero mentioned this pull request Jul 7, 2022
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.

3 participants