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

Fixes for provided apk dependency for instrumentation tests #671

Merged
merged 1 commit into from
Sep 22, 2015

Conversation

kedzie
Copy link
Contributor

@kedzie kedzie commented Aug 8, 2015

-add dummy ZipEntry to apk placeholder. to avoid "0 entries" exception.
-Changed system dependency to use BaseVersion for snapshot versions.

I had issue where the ClasspathModifierLifecycleParticipent used the
full snapshot version with timestamp for the placeholder and system dependency.
But the GenerateSourcesMojo used the SNAPSHOT version without timestamp, resulting in compilation errors. Because actual apk classes.jar wasn't getting added to the classpath.

Note: I experience these problems specifically when there wasn't any artifacts in the local repo. I was doing a verify on an aggregator project, which had both the apk and test projects as modules. So the apk had to come from the previous build, instead of the maven repo. I think this had something to do with the problem. I verified that now it consistently works.

@kedzie
Copy link
Contributor Author

kedzie commented Aug 18, 2015

I think this may be specific to JDK 6. As in the problem is there building with jdk 6

@kedzie
Copy link
Contributor Author

kedzie commented Aug 24, 2015

@psirosky could u look at this real quick? It is a real issue.. I have a couple projects depending on this change.

@mosabua
Copy link
Member

mosabua commented Aug 24, 2015

I am happy to merge this. Ideally with some feedback from @psorobka and cut a new release soon..

@kedzie
Copy link
Contributor Author

kedzie commented Aug 24, 2015

@psorobka Can I get some feedback please? Thanks in advance.

@psorobka
Copy link
Contributor

I'm sorry but I have no idea what is this about.

@kedzie
Copy link
Contributor Author

kedzie commented Sep 5, 2015

There are 2 bugs this solves:

  1. When aar dependencies are added, first a dummy classes.jar is created. And it is created with 0 entries inside it. This causes an exception (I believe only when using JDK 6).
  2. When test apks depend on a SNAPSHOT version of an apk there is a problem in resolving the dependency. This is because the ClasspathModifierLifecycleParticipent used the
    full snapshot version with timestamp for the placeholder and system dependency.
    But the GenerateSourcesMojo used the SNAPSHOT version without timestamp, resulting in compilation errors. Because actual apk classes.jar wasn't getting added to the classpath.

-add dummy ZipEntry to apk placeholder. to avoid "0 entries" exception
-Changed system dependency to use BaseVersion for snapshot versions.
I had issue where the ClasspathModifierLifecycleParticipent used the
full snapshot version with timestamp for the placeholder and system dependency.
But the GenerateSourcesMojo used the SNAPSHOT version without timestamp, resulting in compilation errors.
@kedzie
Copy link
Contributor Author

kedzie commented Sep 11, 2015

@psorobka do you have a better idea of whats it's about?

@kedzie
Copy link
Contributor Author

kedzie commented Sep 21, 2015

@mosabua can you look at this? I promise it is 2 real bugs.

@mosabua
Copy link
Member

mosabua commented Sep 21, 2015

Let me do that tonight..

@mosabua
Copy link
Member

mosabua commented Sep 21, 2015

I will try to release something soon

mosabua added a commit that referenced this pull request Sep 22, 2015
Fixes for provided apk dependency for instrumentation tests
@mosabua mosabua merged commit ac03df5 into simpligility:master Sep 22, 2015
@mosabua
Copy link
Member

mosabua commented Sep 22, 2015

Do we have any filed issues that this closes?

@kedzie
Copy link
Contributor Author

kedzie commented Sep 23, 2015

I never created an issue.. just the PR

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.

3 participants