Added AutoCloseable tests for EntityManagerFactory and EntityManager.

[Tomas-Kraus]

Signed-off-by: Tomas Kraus tomas.kraus@oracle.com

---

name: Pull Request
about: Create a pull request for a Platform TCK change
title: 'Added AutoCloseable tests for EntityManagerFactory and EntityManager'
labels: ''
assignees: ''

---

Fixes Issue
#730

Related Issue(s)
N/A

Describe the change
Added autoCloseableTest into EMF anf EM test clients.

Additional context
N/A

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

[Tomas-Kraus]

Well guys, I'm even not a commiter here. So if someone from TCK team want's to do the cleanup, you are welcome to do that. I just want to add test to verify new API change in JPA 3.1 spec and all I can do is to follow current coding style (which I don't like as well).
I can handle emf/em possible NPE. But I don't think it's a good idea to change how the test is written when noone knows how the new style shall look like.
Edit: But OK, let's give a try to some changes. Here is version without pass variable and with possible NPE fixed.

[Tomas-Kraus]

I'll keep this test as draft until assertion IDs are resolved properly. Asked Alwin for some help with this.

[scottmarlow]

I can handle emf/em possible NPE. But I don't think it's a good idea to change how the test is written when noone knows how the new style shall look like.

👍 on checking for possible NPE.

[scottmarlow]

I'll keep this test as draft until assertion IDs are resolved properly. Asked Alwin for some help with this.

For reference https://github.com/eclipse-ee4j/jakartaee-tck/wiki/Steps-to-generate-the-html-version-of-the-SpecAssertions.xml describes how to generate the assertions.html version of a SpecAssertions.xml input file but I'm not sure of when we need to run that versus manually updating the assertions.html files.

@lukasj should these new AutoCloseable tests use a JavaDoc (for https://github.com/eclipse-ee4j/jpa-api/blob/master/api/src/main/java/jakarta/persistence/EntityManager.java + https://github.com/eclipse-ee4j/jpa-api/blob/master/api/src/main/java/jakarta/persistence/EntityManagerFactory.java) or SPEC (for https://github.com/eclipse-ee4j/jpa-api/blob/master/spec/src/main/asciidoc/ch07-entitymanagers-and-persistence-contexts.adoc) assertion ID?

[lukasj]

@scottmarlow Do you have a pointer to docs describing what these assertions are good for/how they should be written?

[alwin-joseph]

The assertions in the test description seems to be api assertions or javadoc ids. I have added a script in eclipse-ee4j/jakartaee-tck-tools#9 that was used to generate the javadoc assertions from javadocs (Thanks to Jan). Tomas would help trying this script to generate the javadoc ids for jpa.

IMO it would be sufficient to use the api assertion for the existing apis that were modified in this PR. Finding the right javadoc assertion id for the changed methods is required. https://github.com/eclipse-ee4j/jakartaee-tck/blob/master/install/jpa/docs/assertions/api/api-assertions-3_0.html has the api assertions for the previous release.

For reference https://github.com/eclipse-ee4j/jakartaee-tck/wiki/Steps-to-generate-the-html-version-of-the-SpecAssertions.xml describes how to generate the assertions.html version of a SpecAssertions.xml input file but I'm not sure of when we need to run that versus manually updating the assertions.html files.

The https://github.com/eclipse-ee4j/jakartaee-tck/wiki/Steps-to-generate-the-html-version-of-the-SpecAssertions.xml only provides instructions to generate the html version of the already existing spec assertion xml files.

[scottmarlow]

@scottmarlow Do you have a pointer to docs describing what these assertions are good for/how they should be written?

I'll draft something up on https://github.com/eclipse-ee4j/jakartaee-tck/wiki/Guide-to-adding-Specification-Assertions and start a discussion thread for the same on the Platform TCK ML.

[Tomas-Kraus]

I tried to create new API assertions list for 3.1.0 but there was no information about EntityManager/EntityManagerFactory extends AutoCloseable. I have no idea how to add such ID into the list.
Maybe I can just reuse old IDs for EntityManager/EntityManagerFactory close() method which is there since the beginning.

[Tomas-Kraus]

Just keep in mind, that this is JPA 3.1 modification and JPA 3.1 is needed for build/run. Artifacts are available at https://jakarta.oss.sonatype.org/content/repositories/staging/jakarta/persistence/jakarta.persistence-api/3.1.0-RC1/

[scottmarlow]

I tried to create new API assertions list for 3.1.0 but there was no information about EntityManager/EntityManagerFactory extends AutoCloseable. I have no idea how to add such ID into the list. Maybe I can just reuse old IDs for EntityManager/EntityManagerFactory close() method which is there since the beginning.

I built the JPA 3.1 spec source locally and see AutoCloseable mentioned in a few files. In no particular order, below are some SPEC references:

From jpa-api/spec/target/generated-docs/jakarta-persistence-spec-3.1-SNAPSHOT.html#jakarta-persistence-3-1:

A.3. Jakarta Persistence 3.1

EntityManagerFactory and EntityManager interfaces extend java.lang.AutoCloseable interface

From jpa-api/spec/target/generated-docs/jakarta-persistence-spec-3.1-SNAPSHOT.html#a1066 (3.1.1. EntityManager Interface)

public interface EntityManager extends AutoCloseable {

From file:///home/smarlow/work/jakarta/jpa-api/spec/target/generated-docs/jakarta-persistence-spec-3.1-SNAPSHOT.html#entitymanagerfactory-interface (7.4. EntityManagerFactory Interface):

public interface EntityManagerFactory extends AutoCloseable {

I also looked at javadoc via https://jakarta.oss.sonatype.org/content/repositories/staging/jakarta/persistence/jakarta.persistence-api/3.1.0-RC1/jakarta.persistence-api-3.1.0-RC1-javadoc.jar to see possible options for assertions based on that:

The jakarta.persistence/jakarta/persistence/EntityManagerFactory.html has:

All Superinterfaces:
    AutoCloseable

public interface EntityManagerFactory
extends AutoCloseable

The jakarta.persistence/jakarta/persistence/EntityManager.html has:

All Superinterfaces:
    AutoCloseable

public interface EntityManager
extends AutoCloseable

I suggest that api-assertions-3_0.html should have two new rows at the end added with:

ID=PERSISTENCE:JAVADOC:3491

Return=

Method/Field=

Description=EntityManagerFactory must extend java.lang.AutoCloseable

Required=true

Deprecated=

Testable=true

And:

ID=PERSISTENCE:JAVADOC:3492

Return=

Method/Field=

Description=EntityManager must extend java.lang.AutoCloseable

Required=true

Deprecated=

Testable=true

The internal/docs/jpa/Persistence31JavadocAssertions.xml should be updated to include the new IDs (the new tests should be updated to use the new IDs). For the description field, it would be good to say more than I did, if anyone feels creative (perhaps based on wording in https://docs.oracle.com/javase/8/docs/api/java/lang/AutoCloseable.html).

[scottmarlow]

Looks like we should also increment next-available-id in https://github.com/eclipse-ee4j/jakartaee-tck/blob/master/internal/docs/jpa/Persistence22JavadocAssertions.xml#L22 as part of this change.

[scottmarlow]

I'm not sure why PERSISTENCE:JAVADOC:29 is now PERSISTENCE:JAVADOC:29__OLD in internal/docs/jpa/Persistence31JavaDocAssertions.html, @Tomas-Kraus was that a hint that some action needs to be done to the https://github.com/eclipse-ee4j/jakartaee-tck/blob/master/src/com/sun/ts/tests/jpa/core/annotations/entity/Client.java#L65 source that references PERSISTENCE:JAVADOC:29?

It looks like you corrected some entries and kept the original entries with the __OLD name, which seems fine.

[alwin-joseph]

I'm not sure why PERSISTENCE:JAVADOC:29 is now PERSISTENCE:JAVADOC:29__OLD in internal/docs/jpa/Persistence31JavaDocAssertions.html, @Tomas-Kraus was that a hint that some action needs to be done to the https://github.com/eclipse-ee4j/jakartaee-tck/blob/master/src/com/sun/ts/tests/jpa/core/annotations/entity/Client.java#L65 source that references PERSISTENCE:JAVADOC:29?

It looks like you corrected some entries and kept the original entries with the __OLD name, which seems fine.

@Tomas-Kraus The entries with the __OLD name can be removed . We did the same for jaxrs , see jakartaee/rest#1059 (comment)

cc @jansupol

[scottmarlow]

@Tomas-Kraus The entries with the __OLD name can be removed . We did the same for jaxrs , see eclipse-ee4j/jaxrs-api#1059 (comment)

cc @jansupol

I'll push a change to remove the __OLD entries.

[Tomas-Kraus]

PR #793 contains spec assertion IDs update for 3.1. I changed assertion IDs in AutoCloseable tests to point to

• PERSISTENCE:SPEC:2517 - EntityManagerFactory and EntityManager interfaces extend java.lang.AutoCloseable interface

[Tomas-Kraus]

Think we can merge this now.
Regarding __OLD name removal and other manual javadoc lists changes: I don't think any manual action is a good idea here. Process of generating those lists is automated and shall be done in the scripts.