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

Add Parameter wrapper to support older Android versions #3

Merged
merged 1 commit into from Apr 16, 2021
Merged

Add Parameter wrapper to support older Android versions #3

merged 1 commit into from Apr 16, 2021

Conversation

JakeWharton
Copy link
Contributor

Closes #2.

Note: I haven't tested this on an actual pre-26 Android device yet. Will do later tonight or tomorrow. Just wanted to get the conversation started on the workaround.

try {
return Java8.create(method.getParameters());
} catch (NoSuchMethodError ignored) {
return Legacy.create(method);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not always use the Legacy version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Java8 wrapper correctly propagates parameter names (assuming they're available in the class file). I did not look to see how the name is used in practice, though.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The parameter name is to (optionally) format the name of the test:

myTest[myParameter=true]

In any case, I've been investigating whether we can use AnnotationWithMetadata a bit earlier in the process. I'm fine with merging this PR for now and see if I can improve on it at a later point.

@nymanjens
Copy link
Collaborator

Thanks a lot for this PR!

So this PR avoids a dependency on java.lang.reflect.Parameter, which these older Android versions don't support.

FYI: TestParametersMethodProcessor.java also has a dependency on java.lang.reflect.Parameter:

  • TestParametersMethodProcessor is used for @TestParameters.
  • TestParameterAnnotationMethodProcessor edited in this PR is used for @TestParameter.

However, @TestParameters can only work if the parameter names are stored into the jar by the compiler, which normally happens by enabling a command line flag in the Java compiler. I suspect this might in itself be a tricky requirement for the older Android versions?

In any case, I think this runner can be quite useful, even without @TestParameters.

Copy link
Collaborator

@nymanjens nymanjens left a comment

Choose a reason for hiding this comment

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

I asked a colleague who added Anroid support to have a look. Since he's from the US, I'll allow him to get back to me first.

@JakeWharton
Copy link
Contributor Author

I suspect this might in itself be a tricky requirement for the older Android versions?

Even if you opt-in to parameter name retention, Android's toolchain drops the information because there's nowhere to put it in the Dalvik bytecode format. So yeah it sounds like that functionality is going to be a complete non-starter.

@nymanjens nymanjens merged commit 7dccf43 into google:main Apr 16, 2021
@graememorgan
Copy link
Member

LGTM, thanks!

@JakeWharton JakeWharton deleted the jw/older-android-devices/2021-04-14 branch April 16, 2021 15:27
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.

Support for Android pre-API 26
3 participants