Skip to content

Convert hibernate-4.0 tests from groovy to java#9191

Closed
jaydeluca wants to merge 16 commits into
open-telemetry:mainfrom
jaydeluca:hibernate-4-0
Closed

Convert hibernate-4.0 tests from groovy to java#9191
jaydeluca wants to merge 16 commits into
open-telemetry:mainfrom
jaydeluca:hibernate-4-0

Conversation

@jaydeluca

@jaydeluca jaydeluca commented Aug 13, 2023

Copy link
Copy Markdown
Member

Related to #7195

Notes:

  • For one of the existing tests in EntityManagerTest I split out the first test to have the "persist" test case run on it's own to reduce some of the conditionals in the combined test
  • I needed to add the javassist dependency when converting the Spring JPA test due to some errors that looked like: org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'entityManagerFactory' defined in class spring.jpa.PersistenceConfig: Invocation of init method failed; nested exception is java.lang.RuntimeException: error trying to scan <jar-file>:
  • For the EntityManagerTest and the SpringJpaTest there was some logic in the old tests that allowed some conditional span name checks in the groovy versions that made things a little tricky. I added some logic (here and here) to account for them, hopefully it's still clear what is going on but if it's not let me know and I can try and come up with another way.

@jaydeluca jaydeluca requested a review from a team August 13, 2023 12:36
@jaydeluca jaydeluca marked this pull request as draft August 13, 2023 17:22
@jaydeluca jaydeluca marked this pull request as ready for review August 14, 2023 17:13
@breedx-splk

breedx-splk commented Aug 15, 2023

Copy link
Copy Markdown
Contributor

Hey @jaydeluca , thanks for doing this work....and I kinda hate to ask, but it's a pretty large changeset. Is there any chance you could do like one or two test classes at a time across several successive PRs? I think it would make reviewing easier and we can move it through quicker that way.

@jaydeluca

Copy link
Copy Markdown
Member Author

@breedx-splk I would be happy to

@jaydeluca jaydeluca closed this Aug 15, 2023
@breedx-splk

Copy link
Copy Markdown
Contributor

@breedx-splk I would be happy to

Awesome thank you so much!

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.

2 participants