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

Elasticsearch tests are not that well configured #11912

Closed
1 task done
Tcharl opened this issue Jun 6, 2020 · 1 comment
Closed
1 task done

Elasticsearch tests are not that well configured #11912

Tcharl opened this issue Jun 6, 2020 · 1 comment

Comments

@Tcharl
Copy link
Contributor

Tcharl commented Jun 6, 2020

Overview of the issue

Looking at the actual tests with elasticsearch configured is pretty messy:

  • src/test/resources/application.yml reference path.home: it configures an embedded elastic
  • src/test/java//config/ElasticsearchConfiguration overrides that properties setting a temporary folder instead of the fixed folder referenced in the configuration
  • SearchRepositoryMocks are generated in src/test/java//repository/search/SearchRepositoryMock
  • Mocks are used in tests, meaning that the two first bullets are useless
Motivation for or Use Case

Improving the code, avoiding useless properties and behaviors.

Reproduce the error

Generate a platform with elastic

Suggest a Fix

So we have multiple choices here:

  1. Using mock and disabling embedded start: spring boot 2.3 compliant, but with more code (the stub/mock) and potential misbehavior (if the user wants to mock some more method for example)
  2. Using embedded Elastic (thus removing mocks), however, from what I know it's part of Vanroy/JestElastic so will be removed with S.B 2.3. Also, starting elastic before each tests class leads to some strange tests behaviors and sometimes heapdumps see !WIP: Simplify Elasticsearch tests #11903
  3. Using test containers to start embedded elastic: better behavior, but the end user must have docker installed...

I suggest a vote on this ticket, before a PR

JHipster Version(s)

6.9.1

  • Checking this box is mandatory (this is just to show you read everything)
@murdos
Copy link
Contributor

murdos commented Jun 6, 2020

Looking at the actual tests with elasticsearch configured is pretty messy:

* src/test/resources/application.yml reference path.home: it configures an embedded elastic

* src/test/java//config/ElasticsearchConfiguration overrides that properties setting a temporary folder instead of the fixed folder referenced in the configuration

* SearchRepositoryMocks are generated in src/test/java//repository/search/SearchRepositoryMock

* Mocks are used in tests, meaning that the two first bullets are useless

The two first bullets have already been cleaned in #11683 and thus are fixed in the spring-boot-2.3 branch.

3. Using test containers to start embedded elastic: better behavior, but the end user must have docker installed...

I don't think the issue is for end user (docker is already required for development), but rather for CI.
But since that's already the case for Couchbase, Neo4j, Cassandra, Redis, and don't think it's blocking.

@Tcharl Tcharl closed this as completed Jun 7, 2020
@pascalgrimaud pascalgrimaud added this to the 6.10.0 milestone Jun 19, 2020
@murdos murdos removed this from the 6.10.0 milestone Sep 19, 2020
@pascalgrimaud pascalgrimaud added this to the 7.0.0 milestone Oct 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants