jhipster / generator-jhipster

JHipster is a development platform to quickly generate, develop, & deploy modern web applications & microservice architectures.
https://www.jhipster.tech
Apache License 2.0
21.54k stars 4.02k forks source link

Using testcontainers for unit tests instead of h2 mem #17691

Closed Tcharl closed 2 years ago

Tcharl commented 2 years ago
Overview of the feature request

This issue to get vote for moving all test from h2mem (if not selected as a dev database) to their respective testcontainers sql databases

Motivation for or Use Case

Upvote or downvote this thread! I'll make the switch if positive

mraible commented 2 years ago

The downside to moving to Testcontainers is two things, in my experience:

  1. It's a lot slower on a slow internet connection
  2. It causes port conflicts between projects (e.g. you can't run mvn test in parallel)
bsideup commented 2 years ago

Hi @mraible! Let me try answer some of these:

It's a lot slower on a slow internet connection

Testcontainers does not need any internet connection. Docker images need to be downloaded (once), but embedded versions of databases would also require downloading them.

It causes port conflicts between projects (e.g. you can't run mvn test in parallel)

This is exactly what Testcontainers avoids - everything is started on random ports to avoid any conflicts.

https://www.testcontainers.org/features/networking/

Literally in the docs:

From the host's perspective Testcontainers actually exposes this on a random free port. This is by design, to avoid port collisions that may arise with locally running software or in between parallel test runs.

I hope this helps!

atomfrede commented 2 years ago

I would also like to have just the production database instead of h2. When you have started the application at least once and used our docker compose scripts the docker image is available. With regards to CI the connection speed should not be a problem and all providers can cache the images I would say (and with testcontainers cloud it will be again much easier).

Tcharl commented 2 years ago

Still, there will be a huge perf impact on tests when using test containers. The way jh is executing test today is not that accurate and takes some shortcuts, something that we won't be able to achieve anymore. Still, I think that the benefits will overcome the drawbacks, because we'll be able to test on a production-ready like or at least ci-ready like environment locally, particularly for ES and NoSQL dbs which have caused many regressions without us being able to notice them

pascalgrimaud commented 2 years ago

Adding a bounty on this, as it will be important to have.

@Tcharl : anyway, will it be possible to easily switch to h2 for test ? Because during formation, there can be some users under windows with corporate computers without docker? Or is Docker totally mandatory?

Tcharl commented 2 years ago

Hi @pascalgrimaud , yes, it will be very easy: they'll just have to choose devdatabaseType: h2Disk or its in-memory counterpart. The only purpose of this pr is to let the end user to effectively executes his integration tests on the dev (or prod) database he choose instead of defaulting to h2.

Tcharl commented 2 years ago

Bounty claimed: https://opencollective.com/generator-jhipster/expenses/80321

DanielFran commented 2 years ago

@Tcharl approved

michelfigueiredojava commented 2 years ago

unfortunately the tests got too slow

mshima commented 2 years ago

@michelfigueiredojava test using dev profile.

michelfigueiredojava commented 2 years ago

Yes @mshima, I ended up using the dev profile, it makes the tests faster. The downside is having tests connected to the sql db.

Regarding the change discussed in this ticket, I don't think it worth using TestContainers in the tests, because the tests should be fast, "this is one of the key characteristics of SelfTestingCode" (Martin Fowler).

You know the test performance is essential for TDD. For example you are developing a new method in a controller, you can't afford waiting 1 minute for the test to run every time you run your new test.

Tcharl commented 2 years ago

Hi @michelfigueiredojava ,

Do not mix unit tests (which should be fast for sure) that shouldn't be connected to anything (i.e. database is mocked) and integration tests which are made to test the integration of the app within its ecosystem. See the test pyramid for reference These tests should be fast to test for devs (i.e. the dev profile with h2mem) but should be relevant regarding to the production context: that's where testcontainers and prod profile shines.

Additional note: you have many tricks to speed up tests executions with testcontainers (see the video of Josh long and the test container commiter):

bluegaspode commented 2 years ago

In our enviroments only e2e tests run against the production and/or staging database. We are very fine with Integration-Tests, that test the integration of all of our components/software layers within a full SpringBootContext. It doesn't need the production database, H2 has always been very good.

This change also makes integration with GitHub actions more complicated, as we now have to manage caching of the additional containers.

I don't see an option to easily switch back to H2, i.e. the old mode? When I run unit tests in IntelliJ and not via Maven it doesn't work, I have to actively set the profile. With previous JHipster version running JUnit-Tests on default profile (i.e. none actively set) worked fine. Our @SpringBootTests also have to be marked with @IntegrationTest now in addition, otherwise TestContainers don't spin up.

Upgrading to latest JHipster feels cumbersome with this change :(. Doing integration tests with H2 is a very common approach for ages of SW development / Integration Testing.

Tcharl commented 2 years ago

It's pretty easy to switch back to the 'old' behavior: just set h2mem or h2disk as your dev database.

bluegaspode commented 2 years ago

Running tests should not be dependend on my local test database. They should be running on their own.

Tcharl commented 2 years ago

Sorry, but I don't get the answer.

Let me try to describe the logic we have implemented:

bluegaspode commented 2 years ago

I do understand that you optimize for different things like we are.

We optimize for easy/fast tests without docker (i.e. H2) ... with the tradeoff that tests only come close 95% of the real prod environment. Our Dev environment for local testing uses a Prod DB replica. So our unit/integration test environment is not the same as dev/prod environment. JHipster expects it now though.

I do understand, that you optimize for same dev/prod/test environments (and tradeoff some speed for Integration tests).

The test right now don't even start without selecting a profile :( (at least not when started in IntelliJ).

We customized the application.yml of the unit/integration tests back to H2 and removed the dev/prod testing profiles. They just don't make sense for us (based on different tradeoff decisions we took).

mraible commented 2 years ago

The test right now don't even start without selecting a profile :( (at least not when started in IntelliJ).

AFAIK, tests never passed without a profile. Selecting a profile has always been required when using an IDE.

bluegaspode commented 2 years ago

I'm using JHipster since 2016. I've never had to select a profile for running unit tests.

The application.yaml in src/test (at least for relational databases?) had the full H2 profile configured (at least in 7.4, not anymore in 7.9).

pascalgrimaud commented 2 years ago

Sorry @mraible but I don't think so. You could use Spring Initializr, there is no profile, you can still run the test, and there is no profil. The test context will use the application.properties from tests.

This dev/prod profile is specific to JHipster and honestly, I don't like it, because it confuses a lot of users:

That's why in JHipster Lite, dev/prod don't exist anymore

pascalgrimaud commented 2 years ago

Agree with you @bluegaspode I think it's new, since this PR exactly, and for me it's a small breaking change sadly

mraible commented 2 years ago

What I mean to say is whenever I try to run tests in IDEA, I typically have to select a profile. Running gradle test or mvn test doesn't require a profile to be specified.

Y'all might notice I was the first to comment on this PR about speed. I agree it's an issue, but a lot of people seem to love Testcontainers, so I let it go.

Tcharl commented 2 years ago

@bluegaspode thanks for the clarification regarding your use case, it's an additional consideration to take on the framework side (maybe through a dedicated test profile)? Additional question: why not selecting h2mem as your dev database in this case, and use the prod profile with your the same tech as your production datasource? Jhipster won't for sure be able to create your real production topology in any case so it's usually an additional spring's profile to create manually (or environment property override). npm run watch or ./mvnw -Pprod,-webapp && npm serve is usually the good win to produce prod running stuff with velocity

@bluegaspode @pascalgrimaud, you've also spotted a bug: tests should be executed using spring's dev profile by default, without the need to select any maven/gradle profile. To be honest, I do not use mvnw or gradlew cli anymore since the switch to npm has been considered as our CI's default. Still, should work OOTB no matter the cli used.

Are you interested in a fix of one or the other behavior? Someone to create 2 different issues for the follow up? It looks like an easy fix for both on the generator's side

bluegaspode commented 2 years ago

why not selecting h2mem as your dev database in this case and use the prod profile with your the same tech as your production datasource

With the prod profile (I think?) I only get minimized/compressed frontend artifacts. This makes debugging in special situations rather complicated, which is why I use prod profile only for CI/CD builds and - in prod :) Dev profile doesn't use H2, is this allows me to have a replica of Prod-DB. Might be bad style (I know ;), but it helps debugging cornercases before they can be put into proper regression tests.

Are you interested in a fix of one or the other behavior?

I personally can live with my customization now, once I've found out all needed changes :)

As a summary the breaking changes I realized:

1) I have to actively choose a profile, when starting tests in IDEA. Starting via commandline works (the pom.xml sets the testdev profile via command line in maven-failsafe-plugin configuration.) 2) I have to have Docker installed + running, which wasn't required before (or only optionally when running on the 'testcontainers' profile) 3) I had all IntegrationTests just with @SpringBootTest annotation in the past (this was good enough for IntegrationTests). I have to change them to @IntegrationTest (or at least @EmbeddedSQL), as otherwise testcontainers doesn't spin up.

Feel free to decide on your own, how relevant this is for other users or if some small additional documention in the release notes is good enough.

1 can easily be fixed:

src/test/resources/config/application.yml could have a single additional line: spring.profiles.active: testdev

Then it works with starting single tests directly in IDEA (and can still be overriden via commandline).