-
-
Notifications
You must be signed in to change notification settings - Fork 0
GH-127 Add integration tests for each repository #127
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
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThis update adds several new integration test classes for various repository components, each using a MySQL database container to verify saving, retrieving, and removing data. Two existing test classes were updated to explicitly set the database type to MySQL. The build configuration was changed to downgrade the Spigot API and Minecraft versions for compile and runServer tasks, and some dependencies were added to the testImplementation scope. No public APIs were changed outside of the new tests and minor test adjustments. Estimated code review effort3 (~45 minutes) 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (6)
🔇 Additional comments (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (8)
src/test/java/com/eternalcode/parcellockers/database/DeliveryRepositoryIntegrationTest.java (1)
39-39
: Use descriptive test method name.The method name "test" doesn't describe what's being tested. Consider a more descriptive name like
shouldSaveRetrieveAndRemoveDelivery
.-void test() { +void shouldSaveRetrieveAndRemoveDelivery() {src/test/java/com/eternalcode/parcellockers/database/UserRepositoryIntegrationTest.java (3)
29-29
: Use consistent MySQL container setup.This test uses a different MySQL container setup compared to other tests. Consider using the same pattern with
DockerImageName.parse("mysql:latest")
for consistency.-private static final MySQLContainer container = new MySQLContainer(); +private static final MySQLContainer container = new MySQLContainer(DockerImageName.parse("mysql:latest"));
40-40
: Use consistent logger setup.This test passes
null
for the logger while other tests useLogger.getLogger("ParcelLockers")
. Keep it consistent.-DatabaseManager databaseManager = new DatabaseManager(config, null, dataFolder); +DatabaseManager databaseManager = new DatabaseManager(config, Logger.getLogger("ParcelLockers"), dataFolder);
37-37
: Use descriptive test method name.The method name "test" doesn't describe what's being tested. Consider a more descriptive name like
shouldSaveRetrieveAndPageUsers
.-void test() { +void shouldSaveRetrieveAndPageUsers() {src/test/java/com/eternalcode/parcellockers/database/ParcelContentRepositoryIntegrationTest.java (2)
25-25
: Consider using MySQL container for consistency.This test doesn't use a MySQL container like the other integration tests. Consider adding
@Testcontainers
and a MySQL container for consistent database testing.
33-33
: Use descriptive test method name.The method name "test" doesn't describe what's being tested. Consider a more descriptive name like
shouldSaveRetrieveAndRemoveParcelContent
.-void test() { +void shouldSaveRetrieveAndRemoveParcelContent() {src/test/java/com/eternalcode/parcellockers/database/ItemStorageRepositoryIntegrationTest.java (2)
38-38
: Use descriptive test method name.The method name "test" doesn't describe what's being tested. Consider a more descriptive name like
shouldSaveRetrieveAndRemoveItemStorage
.-void test() { +void shouldSaveRetrieveAndRemoveItemStorage() {
48-49
: ItemStack comparison may not work as expected.The
contains
check might fail because ItemStack equality can be tricky. Consider comparing the material and amount separately for more reliable assertions.-assertTrue(retrievedItemStorage.get().items().contains(new ItemStack(Material.GOLD_BLOCK, 64)), - "ItemStorage should contain the saved item"); +var items = retrievedItemStorage.get().items(); +assertTrue(items.size() == 1 && items.get(0).getType() == Material.GOLD_BLOCK && items.get(0).getAmount() == 64, + "ItemStorage should contain the saved item");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
build.gradle.kts
(2 hunks)src/test/java/com/eternalcode/parcellockers/database/DeliveryRepositoryIntegrationTest.java
(1 hunks)src/test/java/com/eternalcode/parcellockers/database/ItemStorageRepositoryIntegrationTest.java
(1 hunks)src/test/java/com/eternalcode/parcellockers/database/LockerRepositoryIntegrationTest.java
(0 hunks)src/test/java/com/eternalcode/parcellockers/database/ParcelContentRepositoryIntegrationTest.java
(1 hunks)src/test/java/com/eternalcode/parcellockers/database/UserRepositoryIntegrationTest.java
(1 hunks)
💤 Files with no reviewable changes (1)
- src/test/java/com/eternalcode/parcellockers/database/LockerRepositoryIntegrationTest.java
src/test/java/com/eternalcode/parcellockers/database/DeliveryRepositoryIntegrationTest.java
Show resolved
Hide resolved
src/test/java/com/eternalcode/parcellockers/database/ItemStorageRepositoryIntegrationTest.java
Show resolved
Hide resolved
src/test/java/com/eternalcode/parcellockers/database/UserRepositoryIntegrationTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Im not good at tests but I think its okey. Good Job Jakub! 😎
5526bf4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks good, check if it works and merge it!
# Conflicts: # build.gradle.kts
No description provided.