go-elasticsearch icon indicating copy to clipboard operation
go-elasticsearch copied to clipboard

feat: use testcontainers-go for the integration tests

Open mdelapenya opened this issue 3 months ago • 2 comments

Hi :wave: from Testcontainers Go community! I'm sending this PR as way to improve the developer experience running the tests in this project, using testcontainers-go.

Of course, it's up to the maintainers whether to include it or not, and I'm happy with any of them, although I'd like to see it included for the reasons explained in the Why is it important? of this PR.

What does this PR do?

This PR adds testcontainers-go as a means to start Elasticsearch containers while running the integration tests.

I'm only adding it to one test function to demonstrate the benefits of declaring the dependencies in the code, compared to use a big set of shell scripts to start the test-time dependencies.

The containertest internal package could be used across different test files containing integration tests, and won't be included in any final binary if not imported from any non-test file.

Why is it important?

It will help contributors to run the integration tests with more ease: just cloning the repo and running make test-integ. The same would apply to the CI.

The code is also self-contained, which means that there is no need to start containers in the CI (Buildkite) in a way that developers need to learn. Here, the dependencies are declared programatically next to the tests, so it's easier to maintain.

Besides that, the plumbing scripts will get reduced, as everything will be done in code, which is easier to test.

Other considerations

I did not configured the Elasticsearch instance as described in the CI shell scripts, and it was on purpose, to showcase how it's possible to start the ES instance with a few lines of code. All the configurations in there can be added to the module thanks to the module configuration API. Please see https://golang.testcontainers.org/modules/elasticsearch/ for more information about the module.

I'm not removing any shell script code (yet), as I'm not that familiar with the codebase, but this PR would be an initial step of a few of them. For that, I'd thank any feedback from your review.

Cheers!

mdelapenya avatar Mar 06 '24 10:03 mdelapenya

That said, integration tests in the client, bulkindexer and several examples could still profit from this.

Yeah, sure. I do not have the knowledge of the codebase to know where testcontainers-go fits the best, so whatever you see, fine for me 🙋

mdelapenya avatar Mar 12 '24 16:03 mdelapenya

@Anaethelion I've resolved the conflicts here. Feel free to ping me for anything you need with this PR.

Cheers!

mdelapenya avatar Apr 24 '24 10:04 mdelapenya