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

refactor/remove-org.reflections.reflections-dependency #87

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

codespearhead
Copy link
Contributor

  • Utilized JPA's Metamodel API to retrieve all managed entity classes.
  • Prefixed instance variables and methods with this. to enhance code clarity.

@appiepollo14 appiepollo14 self-requested a review October 9, 2024 11:19
Copy link
Owner

@appiepollo14 appiepollo14 left a comment

Choose a reason for hiding this comment

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

In general a great PR. I've added a few comments. Please see this link for more references. I believe for now this is a sufficient solution for truncating all tables, however, I still have my doubts whether the DatabaseIntegrationTest is designed as it should.

Another directions would be:

  • Truncate all tables from schema X via a native SQL statement.
  • Rewrite the tests to allow order in the tests so no clearing in between tests has to happen.

@codespearhead codespearhead force-pushed the refactor/remove-org.reflections.reflections-dependency branch from 1123383 to bf72c49 Compare October 15, 2024 18:35
@codespearhead codespearhead marked this pull request as draft October 16, 2024 00:26
@codespearhead
Copy link
Contributor Author

codespearhead commented Oct 16, 2024

If the primary goal of that class is to roll back the database after each test, you can use the io.quarkus.test.TestTransaction annotation instead [1].

Ideally, this annotation should be applied to tests that modify the database. However, I applied it to entire classes, leaving the method-level annotation as a future optimization, since most of the methods change the database anyway.

The issue I'm facing now is that some tests share a state instead of being able to run independently (see "TODO" in ArticlesResourceIntegrationTest.java).

Conversely, the org.junit.jupiter.api.Order annotation can be used to have the tests run in a predictable order. However, it's best not to overuse it so the tests can run in parallel.


[1] https://quarkus.io/guides/getting-started-testing#tests-and-transactions
[2] org.junit.jupiter.api.Order

@codespearhead codespearhead force-pushed the refactor/remove-org.reflections.reflections-dependency branch 2 times, most recently from 366974b to b07bae0 Compare October 19, 2024 17:52
- Utilized JPA's Metamodel API to retrieve all managed entity classes.
- Prefixed instance variables and methods with `this.` to enhance code clarity.
- Rename `AbstractIntegrationTest` to `IntegrationTestUtil`, and make
  its methods public so it can be used without requiring class inheritance.
- Move `createNewArticle` method from `ArticlesResourceIntegrationTest` to `IntegrationTestUtil`.

Signed-off-by: Pedro Aguiar <[email protected]>
@codespearhead codespearhead force-pushed the refactor/remove-org.reflections.reflections-dependency branch from b07bae0 to 7a633bf Compare October 20, 2024 20:56
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