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

Improve usability of JPA TCK #1291

Merged
merged 9 commits into from
Apr 30, 2024

Conversation

beikov
Copy link
Contributor

@beikov beikov commented Apr 25, 2024

Fixes Issue
No issue created for this yet, but I can do so if you need me to. The main goal of this PR is to improve the JPA testing experience by allowing to omit certain system variables when activating certain profiles.

There is also a small README which explains how to run JPA spec tests from within IntelliJ.

Describe the change
See the commit messages

CC @alwin-joseph @anajosep @arjantijms @cesarhernandezgt @dblevins @m0mus @edbratt @gurunrao @jansupol @jgallimore @kazumura @kwsutter @LanceAndersen @bhatpmk @RohitKumarJain @shighbar @gthoman @brideck @OndroMih @dmatej
@starksm64 @scottmarlow

@beikov
Copy link
Contributor Author

beikov commented Apr 25, 2024

By the way, I noticed that the JPA TCK must currently be run with JDK 21, because the Glassfish asadmin command requires JDK 21. I'm seeing the following error:

[INFO] --- exec:3.2.0:exec (start-database) @ persistence-tck-runner ---
Error: LinkageError occurred while loading main class org.glassfish.admin.cli.AsadminMain
        java.lang.UnsupportedClassVersionError: org/glassfish/admin/cli/AsadminMain has been compiled by a more recent version of the Java Runtime (class file version 65.0), this version of the Java Runtime only recognizes class file versions up to 61.0
[ERROR] Command execution failed.

Is that expected or should the final Glassfish version support JDK17 again?

@lukasj
Copy link
Contributor

lukasj commented Apr 25, 2024

the EE Runtime does not have to be GF 8; it can be previous version, WildFly or whatever else...

@dmatej
Copy link
Contributor

dmatej commented Apr 25, 2024

the EE Runtime does not have to be GF 8; it can be previous version, WildFly or whatever else...

This is a usual bug - the exec must respect JAVA used to execute it, while exec doesn't distribute env properties (JAVA_HOME) from the maven build. This way you use JDK21 to run the TCK, but exec will use just any java command found on the file system (PATH). You have to set the env property explicitly like this:

                <plugin>
                    <groupId>org.codehaus.mojo</groupId>
                    <artifactId>exec-maven-plugin</artifactId>
                    <version>3.2.0</version>
                    <configuration>
                        <environmentVariables>
                            <AS_JAVA>${java.home}</AS_JAVA>
                            <JAVA_HOME>${java.home}</JAVA_HOME>
                        </environmentVariables>
                    </configuration>
                </plugin>

Actually we have two branches:

  • 8.0 based on JDK21
  • 8.0-JDK17 for anyone who requires that.

Am I right that the primary target of the JEE11 is Java 21, so everything must run with that too?

@scottmarlow
Copy link
Contributor

Implementations are expected for both JDK 17 + 21.

From https://jakartaee.github.io/platform/jakartaee11/JakartaEE11ReleasePlan:

TCK Source Level

  • A compatible component impl must pass their component TCK when run under Java 17 or 21.
    • To ratify a component specification, there must exist an implementation that passes on Java 17. There must also exist an implementation that passes on 21.
    • These need not be the same implementation. There can be one implementation that passes on 17 and a different one that passes on 21.
  • A compatible platform impl must pass the platform TCK when run under 17 or 21.
    • To ratify a platform specification, there must exist an implementation that passes on 17. There must also exist an implementation that passes on 21.
    • These need not be the same implementation. There can be one implementation that passes on 17 and a different one that passes on 21.

jpa/spec-tests/pom.xml Outdated Show resolved Hide resolved
@beikov beikov force-pushed the tckrefactor-jpa-fixes branch from 51e86a4 to bf8d76d Compare April 26, 2024 16:23
@beikov
Copy link
Contributor Author

beikov commented Apr 26, 2024

If you could merge and publish another persistence-tck ZIP @lukasj, I can test that next week and ensure everything is fine for a release by the end of next week.

@beikov beikov force-pushed the tckrefactor-jpa-fixes branch from bf8d76d to 0edfcb5 Compare April 29, 2024 17:16
@beikov beikov force-pushed the tckrefactor-jpa-fixes branch from 0edfcb5 to 24d55ac Compare April 29, 2024 17:40
@alwin-joseph alwin-joseph merged commit 53add1b into jakartaee:tckrefactor Apr 30, 2024
2 checks passed
<version>${postgresql.jdbc.version}</version>
</dependency>
</dependencies>
</profile>
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm late to review this but this change looks good to me.

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.

5 participants