-
Notifications
You must be signed in to change notification settings - Fork 9
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
50 Adding integration tests #167
Conversation
Integration tests that starts the server on a random free port. Tests for the following commands: PING - PONG SETNX SET - GET DELETE EXISTS Error for not implemented command
Change initialization of ServerSocket setReuseAddress true to happen before binding. Otherwise, the reuse address option doesn't work.
…-lists' into 50-integration-tests # Conflicts: # src/main/java/org/fungover/haze/Main.java # src/test/java/org/fungover/haze/MainTest.java
When running tests testcontainers uses slf4j to do logging. Without an implementation an error message will be printed.
…commands-dont-work-with-lists' into 50-integration-tests # Conflicts: # src/test/java/org/fungover/haze/MainTest.java
Before building new docker images during release integration tests will be run. The command is now mvn -B verify.
SonarCloud Quality Gate failed. |
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.
Good update to erase unnecessary actions like the generation of jar file. The verify with maven is also a good addition. Seems to be some issue with sonar cloud coverage though.
Integration tests that starts the server on a random free port. Tests for the following commands: PING - PONG SETNX SET - GET DELETE EXISTS Error for not implemented command
Change initialization of ServerSocket setReuseAddress true to happen before binding. Otherwise, the reuse address option doesn't work.
When running tests testcontainers uses slf4j to do logging. Without an implementation an error message will be printed.
Before building new docker images during release integration tests will be run. The command is now mvn -B verify.
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.
Looks great! Great to implement integration tests to cover complex methods in main class
The merge-base changed after approval.
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.
Changes looks good with updated versions
The merge-base changed after approval.
conflicts should be resolved by @kappsegla ? |
# Conflicts: # Dockerfile # src/main/java/org/fungover/haze/Main.java
Quality Gate passedKudos, no new issues were introduced! 0 New issues |
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.
this update should compile good, all test checks passed
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.
Looks good!
This will add integration tests that can be used to check that the built docker image can be started and the server called.
The following updates are included:
Changes to the Dockerfile, dependencies are now downloaded here and not by maven.
Github action release.yml now runs mvn verify to verify the application before publishing anything.
Disables generation of a jar file during maven build process because it's not needed. Classes will be copied directly into docker image anyway.