fix tests not behaving consistently with Android #347
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
codecholeric
force-pushed
the
fix-Android-testing-problem
branch
from
28d4413
to
7c4c6ab
Compare
It seems like Gradle Android support treats Java and Android libraries differently. When run from Gradle `ClassLoader.getResources(..)` would return a package entry only from the library JAR, but not the Android JAR. When run from Android Studio it seems like plain file paths and no archives are used, so `ClassLoader.getResources(..)` returns URLs from both libraries.
In the beginning we only used `ClassLoader.getResources(..)`, but then the first bug occurring was that JARs containing a package would not be detected if the folder entry for the package was missing (e.g. look for `java.io` but the entry `/java/io` from the JAR is missing, even though `/java/io/File.class` is there, then `ClassLoader.getResources("/java/io/")` would not give a result). As a workaround we would check if the requested resource was no class file and we got no result and then in turn look through all the JARs on the classpath to return all entries starting with the respective package prefix. Unfortunately this did not work for Android, because `ClassLoader.getResources(..)` provided a partial result, just missing some entries that should be present on the classpath. The only consistent solution seems to be to immediately do the scanning through the classpath ourselves and completely drop the usage of `ClassLoader.getResources(..)`.
I did some performance analysis on Hibernate Core which seemed fine (similar times). What was needed though is a cache for the entries or the build time would explode. This will not make a difference if every location is only scanned once, but as soon as we repeatedly import classes from the classpath the scanning of all archives would cost massive performance (in the end the `ClassLoader` caches all these entries internally as well). I have compared the heap usage with and without the cache on Hibernate core and could not notice a significant increase. Thus it seems okay to go this way and keeping the resource entries in the cache for the lifetime of ArchUnit seems okay.
Signed-off-by: Peter Gafert <[email protected]>
codecholeric
force-pushed
the
fix-Android-testing-problem
branch
from
7c4c6ab
to
7f74b59
Compare
Merged
codecholeric
added a commit
that referenced
this pull request
May 23, 2020
Unfortunately #347 broke the possibility to use a manifest `Class-Path` attribute to specify the classpath. Since this is a feature that many tools use to avoid "command line too long" problems (e.g. on Windows) when the classpath grows in size, I have added support for this to the new way of scanning the classpath. I have also added back the old way to resolve files through the context classloader, since I could not measure a real performance impact and using both sources seems safer in case there is some other classpath quirk I have overlooked. The set will remove duplicates anyway, before starting to import the classes.
codecholeric
added a commit
that referenced
this pull request
Feb 21, 2021
It seems like Gradle Android support treats Java and Android libraries differently. When run from Gradle `ClassLoader.getResources(..)` would return a package entry only from the library JAR, but not the Android JAR. When run from Android Studio it seems like plain file paths and no archives are used, so `ClassLoader.getResources(..)` returns URLs from both libraries.
In the beginning we only used `ClassLoader.getResources(..)`, but then the first bug occurring was that JARs containing a package would not be detected if the folder entry for the package was missing (e.g. look for `java.io` but the entry `/java/io` from the JAR is missing, even though `/java/io/File.class` is there, then `ClassLoader.getResources("/java/io/")` would not give a result). As a workaround we would check if the requested resource was no class file and we got no result and then in turn look through all the JARs on the classpath to return all entries starting with the respective package prefix. Unfortunately this did not work for Android, because `ClassLoader.getResources(..)` provided a partial result, just missing some entries that should be present on the classpath. The only consistent solution seems to be to immediately do the scanning through the classpath ourselves and completely drop the usage of `ClassLoader.getResources(..)`.
I did some performance analysis on Hibernate Core which seemed fine (similar times). What was needed though is a cache for the entries or the build time would explode. This will not make a difference if every location is only scanned once, but as soon as we repeatedly import classes from the classpath the scanning of all archives would cost massive performance (in the end the `ClassLoader` caches all these entries internally as well). I have compared the heap usage with and without the cache on Hibernate core and could not notice a significant increase. Thus it seems okay to go this way and keeping the resource entries in the cache for the lifetime of ArchUnit seems okay.
codecholeric
added a commit
that referenced
this pull request
Feb 21, 2021
Unfortunately #347 broke the possibility to use a manifest `Class-Path` attribute to specify the classpath. Since this is a feature that many tools use to avoid "command line too long" problems (e.g. on Windows) when the classpath grows in size, I have added support for this to the new way of scanning the classpath. I have also added back the old way to resolve files through the context classloader, since I could not measure a real performance impact and using both sources seems safer in case there is some other classpath quirk I have overlooked. The set will remove duplicates anyway, before starting to import the classes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It seems like Gradle Android support treats Java and Android libraries differently. When run from Gradle
ClassLoader.getResources(..)would return a package entry only from the library JAR, but not the Android JAR. When run from Android Studio it seems like plain file paths and no archives are used, soClassLoader.getResources(..)returns URLs from both libraries.In the beginning we only used
ClassLoader.getResources(..), but then the first bug occurring was that JARs containing a package would not be detected if the folder entry for the package was missing (e.g. look forjava.iobut the entry/java/iofrom the JAR is missing, even though/java/io/File.classis there, thenClassLoader.getResources("/java/io/")would not give a result). As a workaround we would check if the requested resource was no class file and we got no result and then in turn look through all the JARs on the classpath to return all entries starting with the respective package prefix. Unfortunately this did not work for Android, becauseClassLoader.getResources(..)provided a partial result, just missing some entries that should be present on the classpath. The only consistent solution seems to be to immediately do the scanning through the classpath ourselves and completely drop the usage ofClassLoader.getResources(..).I did some performance analysis on Hibernate Core which seemed fine (similar times). What was needed though is a cache for the entries or the build time would explode. This will not make a difference if every location is only scanned once, but as soon as we repeatedly import classes from the classpath the scanning of all archives would cost massive performance (in the end the
ClassLoadercaches all these entries internally as well). I have compared the heap usage with and without the cache on Hibernate core and could not notice a significant increase. Thus it seems okay to go this way and keeping the resource entries in the cache for the lifetime of ArchUnit seems okay.Resolves: #319