Skip to content

Add integration-tests module #1123

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

Merged
merged 1 commit into from
Jan 14, 2022
Merged

Conversation

DavideD
Copy link
Member

@DavideD DavideD commented Jan 13, 2022

With a test to check the creation of multiple sessions
from different http requests

This is a draft because it doesn't contain the fix already in main because I want to see that it fails

@DavideD DavideD force-pushed the 1073-integrationtest branch from 19fa76d to 776ae68 Compare January 13, 2022 19:40
@gavinking
Copy link
Member

This is a draft because it doesn't contain the fix already in main because I want to see that it fails

Not sure if that's going to work. Doesn't the CI implicitly try to rebase it? At least that's what I infer happens for ORM.

@DavideD
Copy link
Member Author

DavideD commented Jan 13, 2022

Not sure if that's going to work. Doesn't the CI implicitly try to rebase it? At least that's what I infer happens for ORM.

Damn I didn't know that. It makes sense though. I will temporarly revert those commits on this Draft

@DavideD DavideD force-pushed the 1073-integrationtest branch 4 times, most recently from 4c87bce to 73e4370 Compare January 13, 2022 20:20
@DavideD
Copy link
Member Author

DavideD commented Jan 13, 2022

It seems to work as expected.

A few key points;

  • It only runs for Postgres
  • I've created a separate folder where to add future additional integration tests
  • Right now it runs when the test task is called from the project base dir. It should be fine considering that's only one test and it's not too slow but the next step would be to make it work a bit more like the integration tests with maven (I don't think gradle 6 has a similar taslk)
  • There is some duplication from the test classes in core. It would be nice to be able to reuse them but I'd prefer to treat it as a separate issue
  • I will have to update the README
  • This test really creates 30 entitties with parallels http requests and then reads them in the same way. It's enough to make the test fail without the patch for java.lang.IllegalStateException: Session/EntityManager is closed #1073. It's unlikely but everything could happen so fast and in the right order and the test wouldn't fail. It would be nice to have a simpler way to generate the error. This is a nice test to have anyway

@gavinking
Copy link
Member

That's excellent @DavideD, thanks so much.

@DavideD DavideD force-pushed the 1073-integrationtest branch 2 times, most recently from 4702a0d to dc557c7 Compare January 14, 2022 09:26
With a test to check the creation of multiple sessions
from different http requests
@DavideD DavideD force-pushed the 1073-integrationtest branch from dc557c7 to dcf908f Compare January 14, 2022 09:33
@DavideD DavideD marked this pull request as ready for review January 14, 2022 09:35
@DavideD DavideD merged commit cc2fc5e into hibernate:main Jan 14, 2022
@DavideD DavideD linked an issue Jan 14, 2022 that may be closed by this pull request
@DavideD
Copy link
Member Author

DavideD commented Jan 14, 2022

Merged, thanks everybody

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.

java.lang.IllegalStateException: Session/EntityManager is closed
2 participants