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

Java6Assertions is calling a Java 8 Streams method #1474

Closed
tir38 opened this issue Mar 22, 2019 · 13 comments
Closed

Java6Assertions is calling a Java 8 Streams method #1474

tir38 opened this issue Mar 22, 2019 · 13 comments
Milestone

Comments

@tir38
Copy link

@tir38 tir38 commented Mar 22, 2019

We hit this stack trace on our Android test:

java.lang.NoSuchMethodError: No interface method stream()Ljava/util/stream/Stream; in class Ljava/util/Collection; or its super classes (declaration of 'java.util.Collection' appears in /system/framework/core-libart.jar)
at org.assertj.core.util.Streams.stream(Streams.java:32)
at org.assertj.core.util.Lists.newArrayList(Lists.java:63)
at org.assertj.core.internal.Iterables.assertContainsExactlyInAnyOrder(Iterables.java:1164)
at org.assertj.core.api.AbstractIterableAssert.containsExactlyInAnyOrder(AbstractIterableAssert.java:285)
at org.assertj.core.api.ListAssert.containsExactlyInAnyOrder(ListAssert.java:277)
at org.assertj.core.api.ListAssert.containsExactlyInAnyOrder(ListAssert.java:49)
at com.example.SomeTest_someTestMethod(SomeTest.java:79)

You can see that Streams got added to Android in API 24, so our tests are crashing on earlier versions.

https://developer.android.com/reference/java/util/stream/Stream

We are using the correct Java6 import in our test:

import static org.assertj.core.api.Java6Assertions.assertThat;

@Test
public void someTestMethod() {
	...
    assertThat(actualIds).containsExactlyInAnyOrder(ID_1, ID_2);
}

I don't fully understand how the Java6Assertions package accomplishes it's task, but one of the subsequent calls is not calling purely Java-6 code.

@tir38 tir38 changed the title Java6Assertions is calling a Java 8 Streams method Java6Assertions is calling a Java 8 Streams method Mar 22, 2019
@joel-costigliola
Copy link
Member

@joel-costigliola joel-costigliola commented Mar 23, 2019

Sorry about that, it is likely due to an internal refactoring. which version are you using ?

@epeee
Copy link
Member

@epeee epeee commented Mar 23, 2019

It looks like this one occurs starting from 3.11.0.

@joel-costigliola
Copy link
Member

@joel-costigliola joel-costigliola commented Mar 24, 2019

It's a bit difficult to be aware of android incompatibility since we don't have tests for that.
We have a bunch of assertions that relies on java 8 API (anySatisfy for example), I don't see an easy way to provide Android assertions access only. @PascalSchumacher @benblank thoughts ?

@tir38 the best option is to use assertj-core 2.9.1 which is built with java 7 thus ensuring no java 8 api is used.

@tir38
Copy link
Author

@tir38 tir38 commented Mar 25, 2019

Please forgive my ignorance but why are these assertions accessible via the Java6Assertions class in the first place? Given that ListAssert uses Java8 features, why does Java6Assertions even have an assertThat method that returns a ListAssert? https://github.com/joel-costigliola/assertj-core/blob/master/src/main/java/org/assertj/core/api/Java6Assertions.java#L545

@joel-costigliola
Copy link
Member

@joel-costigliola joel-costigliola commented Mar 25, 2019

I was our attempt at the pre java 8 time to expose assertions for java 6 types only (which List is part of).

Obviously we failed to maintain the java 6 compatibility when adding new assertions for List or simply refactoring the internal implementation. Sorry for that.

Since we are not going to remove java 8 specific assertions from the current version, the only safe way to use AssertJ for Android is to use assertj-core 2.9.1.

We will update the documentation to reflect that.

@PascalSchumacher
Copy link
Member

@PascalSchumacher PascalSchumacher commented Mar 27, 2019

@joel-costigliola
Copy link
Member

@joel-costigliola joel-costigliola commented Mar 28, 2019

Deprecating Java6 assertions classes is a good idea (including documenting what to use instead)
I think once this is done, we can close this issue.

@tir38
Copy link
Author

@tir38 tir38 commented Mar 28, 2019

As part of the documentation cleanup, will you be recommending a "min" Android version for use with 3.x?

Or do you think that 3.x won't support Android until Android fully supports Java8? I.e. it's up to Android to fully support Java8 and all AssertJ's cares about is supporting Java8

@joel-costigliola
Copy link
Member

@joel-costigliola joel-costigliola commented Mar 28, 2019

I don't think we have a 3.x version that supports Android but 2.9.1 is not that old and provides most of the non java 8 assertions in 3.x.

If you find something important missing in 2.9.1, it is always possible to contribute to it and we will release a new version (but otherwise we don't spend time on 2.x).

@joel-costigliola
Copy link
Member

@joel-costigliola joel-costigliola commented Mar 31, 2019

Closing this after deprecating Java 6 assertions entry point classes.

@joel-costigliola joel-costigliola added this to the 3.13 milestone Mar 31, 2019
@tir38
Copy link
Author

@tir38 tir38 commented Jul 11, 2019

"I don't think we have a 3.x version that supports Android but 2.9.1 is not that old and provides most of the non java 8 assertions in 3.x."

Yep so I'm asking if you can update the docs https://assertj.github.io/doc/#assertj-core-java-versions to make it clear that people should use version 2.9.x for all versions of Android?

@PascalSchumacher
Copy link
Member

@PascalSchumacher PascalSchumacher commented Jul 12, 2019

I don't think is correct to say everybody Android developer should use 2.9.x. As you said Stream was added with API Level 24. API level 26 added Path etc., so 3.x (besides SoftAssertions and Assumptions which use the non-Android Byte Buddy variant) should be usable on 26+.

@joel-costigliola
Copy link
Member

@joel-costigliola joel-costigliola commented Jul 14, 2019

@tir38 I have clarified this in the doc, is that better ? see https://assertj.github.io/doc/#assertj-core-android

jadeleeuw added a commit to jadeleeuw/assertj-core that referenced this issue Nov 26, 2019
The goal of these entry points was to provide Java6/Android compatible assertions but since we have moved to Java 8 it became more complicated, filtering out java 8 types is not sufficient as we have refactored some assertions implementation to use lambdas and streams that made them improper to use in Java 6. Reverting all this work is not worth it as Android is moving to have more java 8 support.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

4 participants