search

jakartaee / servlet

Jakarta Servlet
https://eclipse.org/ee4j/servlet
Other
266 stars 87 forks source link

Add a module-info.java #442

Closed arjantijms closed 2 years ago

arjantijms commented 2 years ago

Fixes #201

Module name is jakarta.servlet (Per the naming conventions) Exports the 4 API packages

Opens the 4 API packages as well to prevent errors like:

[ERROR] jakarta.servlet.http.CookieTest.testValue  Time elapsed: 0 s  <<< ERROR!

java.lang.reflect.InaccessibleObjectException: 
    Unable to make jakarta.servlet.http.CookieTest() accessible: 
        module jakarta.servlet does not "opens jakarta.servlet.http" to unnamed module @ffaa6af

Signed-off-by: arjantijms arjan.tijms@gmail.com

markt-asf commented 2 years ago

I'm not sure opening those packages by default is what we want to do. Changing the Surefire Plugin to:

<plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-surefire-plugin</artifactId>
    <version>3.0.0-M5</version>
    <configuration>
        <argLine>--add-opens=jakarta.servlet/jakarta.servlet.http=ALL-UNNAMED</argLine>
    </configuration>
</plugin>

is perhaps a better approach.

I'm also not sure about jakarta.servlet.resources. I'm currently testing this locally to see what - if anything - is required for the schema to be accessible from the API JAR in a JPMS environment.

markt-asf commented 2 years ago

Sorry. Hit the wrong buttons there. We could make everythign public, although I think FindBugs / SpotBugs will then complain. The Surefile config change looks to be the least invasive change at the moment.

I also finished my research on what we need to do for resources. We'll need opens jakarta.servlet.resources; and then the schema will be accessible.

stuartwdouglas commented 2 years ago

The issue here though is that the test is private, and part of the same package, so JUnit can't load the test. Just make the test public.

markt-asf commented 2 years ago

Confirmed. Making the test class and each test method public is sufficient to resolve the error. That solution works for me. (still need to fix resources)

arjantijms commented 2 years ago

There's seemingly a debate surrounding how to go about adding tests to the same Maven module in a modular Java world. The short of it, it's not entirely trivial, and there's a mismatch between Maven and Java modules. Maven distinguishes between regular and test dependencies, while Java modules does not.

Luckily the tests here are simple enough and don't have dependencies of their own.

arjantijms commented 2 years ago

@stuartwdouglas @markt-asf See updated PR