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

fix: Exclude transitive dependencies of espresso-contrib #596

Merged
merged 2 commits into from
Aug 26, 2020
Merged

fix: Exclude transitive dependencies of espresso-contrib #596

merged 2 commits into from
Aug 26, 2020

Conversation

grzegorz-jarosz
Copy link
Contributor

This PR takes forward work started by @tinder-ktarasov in his PR #497, hoping that this will fix issues in #449.

Initial submission had some failing tests (but logs are no longer accessible).

In a short setting "transitive = false" stops gradle from loading espresso-contrib's dependendencies. However, adding it fails one of the tests with some "Navigation" class missing. After adding "com.google.android.material:material:1.2.0" this test no longer failed.

Note that android.material package is in espresson-contrib dependencies, so in fact we're removing all but one of dependent packages from build.

I'm not sure if that's the right "gradle" solution. I'm open for a discussion.

androidTestImplementation "androidx.test.espresso:espresso-contrib:$espresso_version"
androidTestImplementation "androidx.test.espresso:espresso-core:$espresso_version"
androidTestImplementation("androidx.test.espresso:espresso-contrib:$espresso_version") {
transitive = false
Copy link
Contributor

Choose a reason for hiding this comment

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

I would say it makes sense to add a comment here or a link the actual PR with a short explanation about why transitive deps are disabled

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added in f9b1622.

@grzegorz-jarosz
Copy link
Contributor Author

Somehow, I knew it won't be that easy. Okay, back to the drawing board

@mykola-mokhnach
Copy link
Contributor

Somehow, I knew it won't be that easy. Okay, back to the drawing board

What exactly is complicated? The PR itself is ok

@grzegorz-jarosz
Copy link
Contributor Author

Indeed, PR looks simple, but tests are failing for SDK_30 (when I run tests locally they work) and SDK_23 (haven't looked at that one yet)

@mykola-mokhnach
Copy link
Contributor

they were failing even before. It looks like it's partially an azure issue (the emulator is just too slow and times out unexpectedly)

@mykola-mokhnach
Copy link
Contributor

simply add the comment and I would be happy to merge the PR

@grzegorz-jarosz
Copy link
Contributor Author

I've added a comment with short description.

This may not finish all the "Drawable and Lifecycle" issues as there is other PR (#506 ) that took this work further.

In one case I had to use "additionalAppDependencies": ["com.google.android.material:material:1.2.0"] in espressoBuildConfig to solve some issues I had in my case. If I remember correctly those dependencies go into the gradle as "implements".

I guess I'm trying to say that there is a room for improvements.

@mykola-mokhnach
Copy link
Contributor

I guess I'm trying to say that there is a room for improvements.

There always is. Lets do it in small steps.

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.

None yet

2 participants