Remove the need for Docker for a default run [take 2] #386
Conversation
In passing, fix a few corner cases when testing failed deployment.
Note that this requires to index the test classes with Jandex.
For the strict example, it's still possible to test with PostgreSQL by using -Dtest-postgresql (and -Ddocker if you want to start a Docker container).
You need to use -Dtest-mariadb to enable testing and -Ddocker to run the MariaDB server in a Docker container.
You need to use -Dtest-postgresql to enable testing and -Ddocker to run the PostgreSQL server in a Docker container.
It's now the tests that are disabled by default.
There was a problem hiding this comment.
Comments to update contributing.md and to remove some System.out.println.
System.out.println translates to (sometimes unnecessary && hard to control) IO operations which can slower CI runs. Reduction of System.out.println in WildFly codebase helped to speed up and stabilise CI runs, it might not be such a pain in Azure environment.
|
|
||
| Docker is not strictly necessary: it is used to run the MariaDB and PostgreSQL tests which are not enabled by default. However it is a recommended install if you plan to work on Shamrock JPA support: | ||
|
|
||
| * Check [the installation guide](https://docs.docker.com/install/), and [the MacOS installation guide](https://docs.docker.com/docker-for-mac/install/) | ||
| * If you just install docker, be sure that your current user can run a container (no root required). | ||
| On Linux, check [the post-installation guide](https://docs.docker.com/install/linux/linux-postinstall/) |
There was a problem hiding this comment.
Build section could mention some details from description of this PR:
MariaDB and PostgreSQL examples are built by default but not tested. You need to use -Dtest-postgresql
and -Dtest-mariadb to enable testing (+ -Ddocker if you want to rely on the Docker containers).
The strict example is executed with H2 by default but can be executed with PostgreSQL using -Dtest-postgresql.
There was a problem hiding this comment.
Yeah, I wasn't sure we should enter into this level of details here. @cescoffier WDYT?
There was a problem hiding this comment.
I'm wondering if these instructions should not be added to the README of these modules. If we add all the details for all modules, it would quickly become out of track.
There was a problem hiding this comment.
I added a README.md in each example explaining how to run the tests.
| try { | ||
| tcpServer = Server.createTcpServer(); | ||
| tcpServer.start(); | ||
| System.out.println("[INFO] H2 database started in TCP server mode"); |
There was a problem hiding this comment.
can be System.out.println suppressed in this class ? Line 19 + 29
There was a problem hiding this comment.
So, in the end, the idea would be to have the possibility to inject a logger here, e.g. when you are in a Maven build, you could get the Maven logger and log the info properly.
For now, we don't have anything better than this.
| try { | ||
| tcpServer = Server.createTcpServer(); | ||
| tcpServer.start(); | ||
| System.out.println("[INFO] H2 database started in TCP server mode"); |
There was a problem hiding this comment.
can be System.out.println suppressed ? Line 19 + 29
| try { | ||
| tcpServer = Server.createTcpServer(); | ||
| tcpServer.start(); | ||
| System.out.println("[INFO] H2 database started in TCP server mode"); |
There was a problem hiding this comment.
can be System.out.println suppressed in this class ? Line 19 + 29
Supersedes #381 .
Better reviewed commit per commit as I cleaned up a few things.
No docker by default
To start the Docker containers, you now need to use
-Ddocker.MariaDB and PostgreSQL testing disabled by default
MariaDB and PostgreSQL examples are built by default but not tested. You need to use
-Dtest-postgresqland-Dtest-mariadbto enable testing (+-Ddockerif you want to rely on the Docker containers).The strict example is executed with H2 by default but can be executed with PostgreSQL using
-Dtest-postgresql. That's what is done on CI.Test resource management
I added a
@ShamrockTestResourceannotation to be able to declare test resources. The current example in the code is the simple version as I declared the annotations on the lifecycle manager but we could imagine having lifecycle managers in separate jars.I now index the test classes using Jandex to discover these annotations. I think the overhead should be negligible compared to running the tests. Open to discussion if you have better ideas on how to solve this issue.