Add @DataElasticSearchTest support

[mdeinum]

This commit introduces the @DataElasticSearchTest annotation.

See: gh-8544

[snicoll]

Besides the mongdb copy/paste this looks sensible.

[snicoll]

uh? :)

[snicoll]

For consistency could you please use your full first name?

[snicoll]

Ditto

[philwebb]

@snicoll did you mean to put this in the 1.5.x bucket?

[snicoll]

Wrong call, should be 2.0 I suppose.

[philwebb]

It's tempting to try an sneak it into 1.5.x, but 2.0 is better I think.

[snicoll]

OK I've polished the PR (see my branch). @mdeinum could you please configure your IDE to at least use tabs?

So there is one thing I don't like is that the tests create an index and I really don't want that. Discussing with @wilkinsona we think it should probably default to something else with a property to specify it maybe.

If someone wants to work on that, feel free to submit a PR on my fork.

[snicoll]

@philwebb I tried to implement this and it looks like a lot of work. On DataElasticSearchTest we could have a property that specifies if we should attempt to relocate the data directory. We would probably need an extension of SpringBootTestContextBootstrapper to look for the annotation and create a PropertySource with a sensible precedence.

It's not very clear what we should do. Detect a target or build directory and create a sub-directory there? This feels a bit magic to me.

[philwebb]

@snicoll What's creating the data directory? Is it something in ElasticsearchAutoConfiguration? I'm not sure I'm following what needs to happen with the SpringBootTestContextBootstrapper and additional property source. Is it because we need to dynamically detect the directory based on the target/build folder?

[snicoll]

@philwebb our auto-configuration defaults to the current work directory.

So, yes. We need to add a PropertySource where that property is set with a sensible directory. I might argue you probably want a property on the annotation to be able to tune it (and that should create another context if it's changed).

[philwebb]

@snicoll What if we offered some kind of ElasticsearchPropertiesPostProcessor or some plugable strategy that's specific to ElasticsearchAutoConfiguration. That would reduce the problem to register a bean and read some mapped property, hopefully saving the need for any SpringBootTestContextBootstrapper changes.

[snicoll]

Perhaps the main auto-configuration should change then. Two things:

• Nobody wants the indexes generated at the same level of their build file I suppose
• There are cases where you want a fresh index and cases where you don't want to repeat the initialization of the index for every test class

if that was a feature of the auto-configuration, the test auto-config would just flip a switch.

[mdeinum]

In our specific situation, as we used gradle, we put the index in the build directory and recreate before every test. But as that isn't very portable that won't work here. We basically set the path.home using spring.data.elasticsearch.properties.path.home this will then override the default. At least it worked for us.

For testing this could, maybe, be put in the java.io.tmpdir or a sub directory in there instead of the current user.dir?

Could a specific TestExecutionListener be used to override/set the property?

[snicoll]

recreate before every test.

How do you do that?

[mdeinum]

We basically used the sledge hammer approach and tapped into the dirties context support much like the ServletTestExecutionListener. It was better then the approach we had.

Would be nice if we could have something along the lines of @AutoConfigureTestDatabase and replace the client instead of the whole context.

[snicoll]

I am not following. What do you mean by "context"?

[mdeinum]

My bad. When using the dirties context approach a new ApplicationContext is created, which is what I meant with context.

Would it make sense to create an @AutoConfigureTestElasticSearch with a separate namespace (like the @AutoConfigureTestDatabase which could be preconfigured to use a different default for the path.home? and maybe some other defaults to optimize an ElasticSearch instance for testing? maybe with reusing parts of the current ElasticSearchAutoConfiguration?

[snicoll]

@mdeinum you're the one submitting this PR so if you know good defaults, by all means share them.

[snicoll]

@mdeinum are you going to refine your PR with what we've discussed here?

[wilkinsona]

In Elasticsearch 5 they have dropped official support for running embedded Elasticsearch. They do, however, provide EsIntegTestCase. Unfortunately, it's very picky about what's on the classpath and tests fail to launch if it finds any duplicate classes. For example it pulls in org.elasticsearc:securemock which contains lots of Mockito classes. This then prevents the use of Mockito itself within the same project. Even with lots of exclusions in place to tune the classpath, it still couldn't run a unit test in Eclipse as the check found a duplicate internal Eclipse class.

The alternative is to roll our own code to bootstrap an Elasticsearch node as part of @DataElasticsearchTest that skips the jar hell check. That would probably be sufficient for integration testing purposes.

[mdeinum]

That is too bad :( . I'm currently a bit swamped in work and finishing a book. If all goes well I probably have time next week to take another look at this.

Could we take some inspiration from the EsIntegTestCase (this is what I initially did for the 2.x version).

[wilkinsona]

It looks like this has been scuppered by the current situation with embedding Elasticsearch and the severe restrictions of EsIntegTestCase. It doesn't look like that situation is going to change in the foreseeable future so, with some regret, I'm going to close this. Thanks anyway, @mdeinum. If another approach comes to light, we can resurrect this PR.

[hantsy]

Oh, this PR finally missed merging into Spring Boot?

[wilkinsona]

Yes. Please see the various comments above for the reasons why it could not be merged.