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

Don't kill/recreate containers on every integration/product test run on local machine #4514

Closed
iirekm opened this issue Jul 21, 2020 · 9 comments

Comments

@iirekm
Copy link

iirekm commented Jul 21, 2020

  • it would make fix-run-fix-run... cycle on developer's machine faster; TestContainers slows it down by recreating the containers all the time
  • sometimes it's useful to inspect or modify the container manually (eg what's inside the container after a failed test); the container killing makes it impossible

During GHA build this functionality should be disabled.

It should be possible with withReuse functionality recently added to TestContainers (testcontainers/testcontainers-java#1781 )

The same issue is with product tests, there we probably need to hook into Environment.start

@iirekm iirekm changed the title Don't kill/recreate containers on every integration/product test run on local machine Don't kill/recreate containers on every integration test run on local machine Jul 21, 2020
@kokosing
Copy link
Member

It should be possible with withReuse functionality

I played a bit with withReuse and it was not working well. It requires change in the code and in local environment.

Maybe env up command could create additional containers that could be in stand-by mode. When need you could run tempto there or presto-cli. What do you think?

@findepi any ideas?

@findepi
Copy link
Member

findepi commented Jul 21, 2020

I feel strongly that container reuse must be an opt in. The default behavior must be KISS and same on CI (GHA) and local environment.
Any shortcut or clever container reuse can affect test results, so I want to know I am using it (opt in) instead of wondering what is going on and how to fix it (opt out).

Maybe env up command could create additional containers that could be in stand-by mode. When need you could run tempto there or presto-cli. What do you think?

Containers are used in quite few different kinds of tests today.
Product tests, connectors integrations tests, and "hive tests".
Different cases have different potential benefits and implementation costs.

Also, for product tests, there is a known (but undocumented way) or starting
the containers once (env up) and running tests directly from the IDE.
(This works for some tests, but not all, today.)

I think we could start by defining the objectives we want to achieve first.

@iirekm
Copy link
Author

iirekm commented Aug 13, 2020

This is in progress. presto-products-tests-launcher is unnecessarily complicated, but I managed to reuse containers also for product tests and for env up.

@kokosing
Copy link
Member

FYI: #3662

@iirekm
Copy link
Author

iirekm commented Aug 14, 2020

For product tests - done: https://github.com/prestosql/presto/pull/4834/files

For integration tests - needs some changes in tests, will do it soon

@iirekm iirekm changed the title Don't kill/recreate containers on every integration test run on local machine Don't kill/recreate containers on every integration/product test run on local machine Aug 18, 2020
@findepi
Copy link
Member

findepi commented Aug 21, 2020

I am actually not sure we need any changes to how product test environment is started.

We have env up command, which is currently very useful:

  • it allows me to use the test env to manually reproduce some issue
  • (with some additional trick) it allows me to run a product test from my IDE

What it misses today is:

  • allow running a product test the normal way (inside docker, inside env's network)

We should add this.

(#4911)

@kokosing
Copy link
Member

One might say this pull request implements #4911.

I think we need to discuss this.

@findepi
Copy link
Member

findepi commented Aug 21, 2020

One might say this pull request implements #4911.

I do not yet see it this way.

@hashhar
Copy link
Member

hashhar commented Sep 3, 2024

We now have container reuse for non-product tests using testcontainers. TESTCONTAINERS_REUSE_ENABLE.

For product tests we can start product test env on CLI and run tests which reuse them using IDE.

I'm closing this since in practice it's not been very useful or widely used.

We can reopen if we need to in future.

@hashhar hashhar closed this as not planned Won't fix, can't repro, duplicate, stale Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

4 participants