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

Java 9 support #47

Closed
MaxKiesel opened this issue Dec 1, 2017 · 11 comments
Closed

Java 9 support #47

MaxKiesel opened this issue Dec 1, 2017 · 11 comments
Assignees
Milestone

Comments

@MaxKiesel
Copy link

With Java 9, the application class loader isn't an instance of URLClassLoader any more, but ArchUnit searches for a class loader of this type.
So ArchUnit doesn't find any classes in class path.
So no classes are checked and all tests pass successfully.
ArchUnit must be modified to support both Java 8 or lower and Java 9 class loading mechanism.

@codecholeric
Copy link
Collaborator

Yes, unfortunately at the moment ArchUnit doesn't support Java 9 (yet). This is not only the way it detects classes on the classpath (relying on URLClassLoader's API), but also the parsing of the bytecode wouldn't work (so importing a Java 9 class at the file level doesn't work either).
So far the biggest problem is, that ArchUnit is compatible with Java 7 to support late adopting enterprise projects. And up to now I wanted to keep it simple with the branches.
But probably I have to start making different branches to support different versions (e.g. JUnit 5 with Java 8, and an adjusted one with the classpath handling for Java 9).
The only other way would be, to modularize the actual class import and plug in different versions, but then with respect to JUnit 5, which needs Java 8, the old Java version is a problem again.
I'll think about this, and then start working on this, as soon as I can...

@codecholeric
Copy link
Collaborator

I've pushed a first attempt to support Java 9, can you test this? You have to add the SNAPSHOT repository:

<repositories>
    <repository>
        <id>sonatype-snapshots</id>
        <url>https://oss.sonatype.org/content/repositories/snapshots</url>
    </repository>
</repositories>

and include the Java 9 Dependency:

<dependencies>
    <dependency>
        <groupId>com.tngtech.archunit</groupId>
        <artifactId>archunit-junit</artifactId>
        <version>0.6.0-java9-SNAPSHOT</version>
        <scope>test</scope>
    </dependency>
</dependencies>

@MaxKiesel
Copy link
Author

With this hotfix, the class files of my Java9 project are scanned and broken architecture rules are reported by ArchUnit tests correctly.
So for my concerns, with this hotfix, the Java9 support is sufficient.
Thank you very much!

@codecholeric
Copy link
Collaborator

Okay, since I haven't gotten any negative feedback, I'll merge this back into master in a couple of days. So if anyone has a reason why this approach doesn't work, speak now or forever hold your peace 😉

@jqno
Copy link
Contributor

jqno commented May 16, 2018

Is there a release planned for this? I want to use ArchUnit but can't because my project is on Java 9.

@codecholeric codecholeric added this to the 0.8.0 milestone May 16, 2018
@codecholeric
Copy link
Collaborator

I have to admit, I'm feeling the urge to shout out some excuses like "deadlines in my project" or my bad idea to release this together with JUnit 5 support, but I'll refrain from it 😉

You're absolutely right, this should have been released long ago, so I proudly present ArchUnit 0.8.0
https://github.com/TNG/ArchUnit/releases/tag/v0.8.0

I'd be happy about any feedback,

  • does the import of classes from different modules work as expected (it should pretty much be the same behavior as with older versions, i.e. it just imports classes from all modules)
  • should the ClassFileImporter learn a new concept, e.g. JavaModules containing JavaClasses grouped by module, so one can specify rules about the relationship of modules and their classes to each other
  • any other thing you're missing, or things that feel awkward if Jigsaw is added to the puzzle...

Unfortunately my bread and butter is in legacy, so I have no big real life project at hand, where I could try out, how this feels with many big Java 9 modules, etc.

@jqno
Copy link
Contributor

jqno commented May 17, 2018

Wow, that was fast! Thanks! 😃
I have to be honest, my project isn't properly modularized yet, and won't be for a little while longer (it's a library that also still has to target Java 8). I have played with ArchUnit 0.8.0 a little bit and I can at least confirm that it now works fine when run on JDK 9 and JDK 10.
I'm not sure to what extent the ClassFileImporter needs to learn about modules, because the whole point of the module system is that the compiler can now enforce which classes can or can't be accessed by other modules. I don't have enough experience with either ArchUnit or the module system yet to be able to give you a good answer to that. But I'm certainly willing to file an issue if I run into something.
In the mean time, I'm very happy with the fact that I can start adopting ArchUnit in my project. Thank you!

@codecholeric
Copy link
Collaborator

Glad to hear that it works 😃
Honestly that was my impression anyway, that the Java ecosystem will still need some time to move forward to Jigsaw. I have the same problem with ArchUnit itself, since I want to stay compatible with JDK 7.
I agree with your point about Jigsaw modules, however I could imagine, that there might be properties that the compiler won't assert, but that still would make sense to assert for certain projects. But I'll wait for any real life examples for that...
You can use ArchUnit to move towards modularization, and enforce the same properties that Jigsaw could (i.e. preparing a simple migration, once you really move to Jigsaw)
Anyway, feel free to report any issue you encounter!!

@jqno
Copy link
Contributor

jqno commented May 17, 2018

You could consider adding an Automatic-Module-Name to your Manifest: http://branchandbound.net/blog/java/2017/12/automatic-module-name/
That will make it easier for projects that are trying to modularize, to consume your library. (Although to be honest, I'm not sure if it matters that much for a testing library.)

I'll let you know if I encounter anything. In the mean time, Travis has just reported successful builds on various JVMs for my first ArchUnit tests: https://travis-ci.org/jqno/equalsverifier/builds/380319383 🎉

@codecholeric
Copy link
Collaborator

Great 😃
I think I've already added the Automatic-Module-Name Manifest entry, are you missing it from your dependency? Cause I've just checked Maven Central and it seems to be in order... (I remember adding that to the Gradle build...)

@jqno
Copy link
Contributor

jqno commented May 18, 2018

I'm sorry, you're right, it's there. 👍

hankem added a commit that referenced this issue Nov 13, 2022
Bumps \[gradle/wrapper-validation-action\](https://github.com/gradle/wrapper-validation-action) from 1.0.4 to 1.0.5.

# Release notes

from [gradle/wrapper-validation-action's releases](https://github.com/gradle/wrapper-validation-action/releases):

> ## v1.0.5
> ------
> 
> ### Gradle Wrapper Validation
> 
> *   Update dependencies for Node 16 ([#53](https://github-redirect.dependabot.com/gradle/wrapper-validation-action/issues/53))
> *   Update dependencies with security vulnerabilities ([#67](https://github-redirect.dependabot.com/gradle/wrapper-validation-action/issues/67))
> *   Update various other dependencies ([#45](https://github-redirect.dependabot.com/gradle/wrapper-validation-action/issues/45), [#47](https://github-redirect.dependabot.com/gradle/wrapper-validation-action/issues/47), [#48](https://github-redirect.dependabot.com/gradle/wrapper-validation-action/issues/48), [#54](https://github-redirect.dependabot.com/gradle/wrapper-validation-action/issues/54))

# Commits

*   [`55e685c`](gradle/wrapper-validation-action@55e685c) Dependencies
*   [`f232d2e`](gradle/wrapper-validation-action@f232d2e) Merge branch 'master' into releases/v1
*   [`9aa31f2`](gradle/wrapper-validation-action@9aa31f2) Merge pull request [#67](https://github-redirect.dependabot.com/gradle/wrapper-validation-action/issues/67) from gradle/dd/update-deps
*   [`5d6ea91`](gradle/wrapper-validation-action@5d6ea91) Extend timeout for Jest tests
*   [`db7df1f`](gradle/wrapper-validation-action@db7df1f) Update dependencies with vulnerabilities
*   [`859c332`](gradle/wrapper-validation-action@859c332) Merge pull request [#53](https://github-redirect.dependabot.com/gradle/wrapper-validation-action/issues/53) from KengoTODA/node-16
*   [`d0ffc95`](gradle/wrapper-validation-action@d0ffc95) ci: install node v16 instead of v12
*   [`6793660`](gradle/wrapper-validation-action@6793660) Merge remote-tracking branch 'upstream/master' into node-16
*   [`781fa15`](gradle/wrapper-validation-action@781fa15) Merge pull request [#54](https://github-redirect.dependabot.com/gradle/wrapper-validation-action/issues/54) from gradle/dependabot/npm\_and\_yarn/ansi-regex-5.0.1
*   [`7606dd0`](gradle/wrapper-validation-action@7606dd0) Bump ansi-regex from 5.0.0 to 5.0.1

Additional commits viewable in [compare view](gradle/wrapper-validation-action@v1.0.4...v1.0.5)
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

3 participants