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

JUnit5 5.4.0 broke Android support #1800

Closed
matejdro opened this issue Mar 5, 2019 · 14 comments
Closed

JUnit5 5.4.0 broke Android support #1800

matejdro opened this issue Mar 5, 2019 · 14 comments

Comments

@matejdro
Copy link

matejdro commented Mar 5, 2019

JUnit 5.4.0 started crashing on Android due to unsupported UNICODE_CHARACTER_CLASS flag in Pattern.

Steps to reproduce

  1. Create brand new emulator from Android Studio with Android 9.0 (Google Play) x86_64 image
  2. checkout https://github.com/mannodermaus/android-junit5/tree/7b9f490d9ba6e7eb6b91b99e6d2755c546354e4f
  3. Start emulator
  4. Run ./gradlew :sample:connectedExperimentalDebugAndroidTest from the root of checked out repo
  5. Tests will crash with following exception:
java.lang.ExceptionInInitializerError
at org.junit.platform.commons.util.StringUtils.isNotBlank(StringUtils.java:64)
at org.junit.platform.commons.util.Preconditions.notBlank(Preconditions.java:248)
at org.junit.platform.launcher.core.LauncherConfigurationParameters.<init>(LauncherConfigurationParameters.java:44)
at org.junit.platform.launcher.core.LauncherConfigurationParameters.<init>(LauncherConfigurationParameters.java:39)
at org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder.build(LauncherDiscoveryRequestBuilder.java:194)
at org.junit.platform.runner.JUnitPlatform.createDiscoveryRequest(JUnitPlatform.java:153)
at org.junit.platform.runner.JUnitPlatform.<init>(JUnitPlatform.java:124)
at org.junit.platform.runner.JUnitPlatform.<init>(JUnitPlatform.java:117)
at de.mannodermaus.junit5.AndroidJUnit5.<init>(Runner.kt:17)
at de.mannodermaus.junit5.RunnerKt.createJUnit5Runner(Runner.kt:22)
at de.mannodermaus.junit5.AndroidJUnit5Builder.runnerForClass(RunnerBuilder.kt:69)
at org.junit.runners.model.RunnerBuilder.safeRunnerForClass(RunnerBuilder.java:59)
at androidx.test.internal.runner.AndroidRunnerBuilder.runnerForClass(AndroidRunnerBuilder.java:147)
at org.junit.runners.model.RunnerBuilder.safeRunnerForClass(RunnerBuilder.java:59)
at androidx.test.internal.runner.TestLoader.doCreateRunner(TestLoader.java:73)
at androidx.test.internal.runner.TestLoader.getRunnersFor(TestLoader.java:104)
at androidx.test.internal.runner.TestRequestBuilder.build(TestRequestBuilder.java:789)
at androidx.test.runner.AndroidJUnitRunner.buildRequest(AndroidJUnitRunner.java:543)
at androidx.test.runner.AndroidJUnitRunner.onStart(AndroidJUnitRunner.java:386)
at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:2145)
Caused by: java.lang.IllegalArgumentException: Unsupported flags: 256
at java.util.regex.Pattern.<init>(Pattern.java:1324)
at java.util.regex.Pattern.compile(Pattern.java:975)
at org.junit.platform.commons.util.StringUtils.<clinit>(StringUtils.java:36)
... 20 more

It looks like UNICODE_CHARACTER_CLASS was added to StringUtils in JUnit 5.4.0. This flag is not supported on Android (not even on the latest devices) and causes crash whenever StringUtils class is initialized.

@sormuras
Copy link
Member

sormuras commented Mar 5, 2019

I remember that late change in line db8b3d4#diff-6620c2fff8288084514a095acc4cb5a8R36 ... used another in-line regular expression before.

@sormuras
Copy link
Member

sormuras commented Mar 5, 2019

The constant is available since Java 1.7, though. Copied from java.util.regex.Pattern#UNICODE_CHARACTER_CLASS:

    /**
     * Enables the Unicode version of <i>Predefined character classes</i> and
     * <i>POSIX character classes</i>.
     *
     * <p> When this flag is specified then the (US-ASCII only)
     * <i>Predefined character classes</i> and <i>POSIX character classes</i>
     * are in conformance with
     * <a href="http://www.unicode.org/reports/tr18/"><i>Unicode Technical
     * Standard #18: Unicode Regular Expression</i></a>
     * <i>Annex C: Compatibility Properties</i>.
     * <p>
     * The UNICODE_CHARACTER_CLASS mode can also be enabled via the embedded
     * flag expression&nbsp;{@code (?U)}.
     * <p>
     * The flag implies UNICODE_CASE, that is, it enables Unicode-aware case
     * folding.
     * <p>
     * Specifying this flag may impose a performance penalty.  </p>
     * @since 1.7
     */
    public static final int UNICODE_CHARACTER_CLASS = 0x100;

@sormuras
Copy link
Member

sormuras commented Mar 5, 2019

Reading the documention again...

     * The UNICODE_CHARACTER_CLASS mode can also be enabled via the embedded
     * flag expression&nbsp;{@code (?U)}.

...(?U) is the pattern snippet, aka flag expression.

@sormuras
Copy link
Member

sormuras commented Mar 5, 2019

@matejdro Does Android support (?U)?

@matejdro
Copy link
Author

matejdro commented Mar 5, 2019

The constant is available since Java 1.7, though

Yes, the constant itself is available. Unfortunately it appears Android folks forgot to update validation code, so this constant is still not allowed to be passed into Pattern constructor.

Does Android support (?U)?

Just checked and Android 9.0 (latest one at the moment) throws syntax error when I try to create Pattern with this text added in. On the bright side, this mode appears to be always enabled on Android according to Android Documentation.

@sormuras
Copy link
Member

sormuras commented Mar 6, 2019

Thanks for checking @matejdro -- mh, maybe we have to resort to only look for non-Unicode control chars on Android. Perhaps we'll enhance the OS enumeration (is it possible to determine Android OS at runtime?) with an ANDROID constant.

Marked for @junit-team discussion.

@sormuras
Copy link
Member

sormuras commented Mar 6, 2019

...or guard the compilation of the patterns in StringUtils https://github.com/junit-team/junit5/blob/master/junit-platform-commons/src/main/java/org/junit/platform/commons/util/StringUtils.java#L36-L37 in a static initializer and fall-back to a non-Unicode one.

@ghost ghost removed the status: team discussion label Mar 8, 2019
@marcphilipp marcphilipp modified the milestones: 5.5 M1, 5.4.1 Mar 8, 2019
@sormuras
Copy link
Member

sormuras commented Mar 9, 2019

@matejdro @mannodermaus With 30ab9f4 applied Android support should be fixed.

Can you please double-check on your systems using latest SNAPSHOT of JUnit 5?

https://oss.sonatype.org/content/repositories/snapshots/org/junit/

marcphilipp pushed a commit that referenced this issue Mar 9, 2019
`java.util.regex.Pattern.UNICODE_CHARACTER_CLASS` isn't supported on
all runtime platforms, like Android. This commit guards the compilation
of the regex pattern by catching `IllegalArgumentException` -- and
falling back to match only ASCII control characters.

Note: according to https://developer.android.com/reference/java/util/regex/Pattern.html#UNICODE_CHARACTER_CLASS
the Unicode mode is always enabled on Android. Thus all of the
characters that Unicode refers to as 'control characters' are still
matched on Android.

Fixes #1800
@marcphilipp
Copy link
Member

I've cherry-picked 30ab9f4 to the releases/5.4.x branch.

@mannodermaus
Copy link

Perhaps we'll enhance the OS enumeration (is it possible to determine Android OS at runtime?) with an ANDROID constant.

The way this is usually checked by multi-platform libraries is through a reflective class lookup for android.os.Build, followed by a conditional around the android.os.Build.Version.SDK_INT constant. Here is an example of that!

@matejdro
Copy link
Author

I've just checked and 5.5.0-SNAPSHOT works on Android!

Thanks a lot.

@sormuras
Copy link
Member

Thanks for the heads-up, @mannodermaus and @matejdro!

Regarding OS.ANDROID ... how is the relation to OS.LINUX? Does OS.currentOs() report LINUX on Android at the momemt? If we were to introduce OS.ANDROID would that also imply OS.LINUX? Perhaps we a secondary attribute/enum, like OS.Flavor... or something.

The fix for Android will be released soon with version 5.4.1.

@mannodermaus
Copy link

Indeed, OS.LINUX.isCurrentOs() is true when running on an Android device:

@Test
fun testOsCurrent() {
    val currentOs = OS.values().first { it.isCurrentOs }
    Log.i("AndroidLog", "Current OS: $currentOs")
    Log.i("AndroidLog", "os.name: ${System.getProperty("os.name")}")
    Log.i("AndroidLog", "java.vm.name: ${System.getProperty("java.vm.name")}")
}
Output:
I/AndroidLog: Current OS: LINUX
I/AndroidLog: os.name: Linux
I/AndroidLog: java.vm.name: Dalvik

If we wanted to stay with system property lookups, java.vm.name might be another good candidate to detect Android, I suppose. Then again, this would warrant its own ticket and discussion!

@sbrannen
Copy link
Member

If we wanted to stay with system property lookups, java.vm.name might be another good candidate to detect Android, I suppose. Then again, this would warrant its own ticket and discussion!

Agreed. Please open a new ticket if you want to follow up on Android OS detection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants