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

Comments

Projects
None yet
4 participants
@tir38
Copy link

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

This comment has been minimized.

Copy link
Owner

commented Mar 23, 2019

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

@epeee

This comment has been minimized.

Copy link
Collaborator

commented Mar 23, 2019

It looks like this one occurs starting from 3.11.0.

@joel-costigliola

This comment has been minimized.

Copy link
Owner

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Owner

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

This comment has been minimized.

Copy link
Collaborator

commented Mar 27, 2019

@joel-costigliola

This comment has been minimized.

Copy link
Owner

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Owner

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

This comment has been minimized.

Copy link
Owner

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Collaborator

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

This comment has been minimized.

Copy link
Owner

commented Jul 14, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.