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

fix: terminate etcd process after running tests #2359

Merged
merged 1 commit into from
May 31, 2023

Conversation

chahn
Copy link
Contributor

@chahn chahn commented May 31, 2023

After the etcd tests are done, etcd is not terminated.
Subsequent tests may therefore fail because an etcd process is still running and a new process of etcd can't start.
For example, if tests are run locally.

Executing os.Exit() terminates the program immediately; the deferred etcdtest.Stop() function is not executed [1].
The provided patch would terminate the etcd process in the end.

[1]: https://pkg.go.dev/os#Exit

@szuecs
Copy link
Member

szuecs commented May 31, 2023

👍

1 similar comment
@AlexanderYastrebov
Copy link
Member

👍

@AlexanderYastrebov AlexanderYastrebov merged commit b4df29a into zalando:master May 31, 2023
@chahn
Copy link
Contributor Author

chahn commented May 31, 2023

Thanks a lot for merging!

AlexanderYastrebov added a commit that referenced this pull request Jul 11, 2023
Use ./... instead of loop over packages.

Originally loop over packages was added by #359 that introduced glide package manager.
This manager fetches dependencies into vendor/ subdirectory (see https://glide.readthedocs.io/en/latest/getting-started/) and
apparently loop over packages was nessesary back then.
glide usage was removed by #764

The change now also runs testable examples.

Also:
* fixes cmd/eskip etcd shutdown similar to #2359
* fixes cmd/eskip test intruduced by #2202
* fixes lua example
* removed bench target as it makes little sense
* .coverprofile-all can not be simplified yet due to golang/go#45361

Signed-off-by: Alexander Yastrebov <[email protected]>
AlexanderYastrebov added a commit that referenced this pull request Jul 11, 2023
Use ./... instead of loop over packages.

Originally loop over packages was added by #359 that introduced glide package manager.
This manager fetches dependencies into vendor/ subdirectory (see https://glide.readthedocs.io/en/latest/getting-started/) and
apparently loop over packages was nessesary back then.
glide usage was removed by #764

The change now also runs testable examples.

Also:
* fixes cmd/eskip etcd shutdown similar to #2359
* fixes cmd/eskip test intruduced by #2202
* fixes lua example
* removed bench target as it makes little sense
* .coverprofile-all can not be simplified yet due to golang/go#45361

Signed-off-by: Alexander Yastrebov <[email protected]>
AlexanderYastrebov added a commit that referenced this pull request Jul 11, 2023
Use ./... instead of loop over packages.

Originally loop over packages was added by #359 that introduced glide package manager.
This manager fetches dependencies into vendor/ subdirectory (see https://glide.readthedocs.io/en/latest/getting-started/) and
apparently loop over packages was necessary back then.
glide usage was removed by #764

The change now also runs testable examples.

Also:
* fixes cmd/eskip etcd shutdown similar to #2359
* fixes cmd/eskip test intruduced by #2202
* fixes lua example
* removed bench target as it makes little sense
* .coverprofile-all can not be simplified yet due to golang/go#45361

Signed-off-by: Alexander Yastrebov <[email protected]>
AlexanderYastrebov added a commit that referenced this pull request Jul 11, 2023
Use ./... instead of loop over packages.

Originally loop over packages was added by #359 that introduced glide package manager.
This manager fetches dependencies into vendor/ subdirectory (see https://glide.readthedocs.io/en/latest/getting-started/) and
apparently loop over packages was necessary back then.
glide usage was removed by #764

The change now also runs testable examples.

Also:
* fixes cmd/eskip etcd shutdown similar to #2359
* fixes cmd/eskip test introduced by #2202
* fixes lua example
* removed bench target as it makes little sense
* .coverprofile-all can not be simplified yet due to golang/go#45361

Signed-off-by: Alexander Yastrebov <[email protected]>
AlexanderYastrebov added a commit that referenced this pull request Jul 11, 2023
Use ./... instead of loop over packages.

Originally loop over packages was added by #359 that introduced glide package manager.
This manager fetches dependencies into vendor/ subdirectory (see https://glide.readthedocs.io/en/latest/getting-started/) and
apparently loop over packages was necessary back then.
glide usage was removed by #764

The change now also runs testable examples.

Also:
* fixes cmd/eskip etcd shutdown similar to #2359
* fixes cmd/eskip test introduced by #2202
* fixes lua example
* removed bench target as it makes little sense
* .coverprofile-all can not be simplified yet due to golang/go#45361

Signed-off-by: Alexander Yastrebov <[email protected]>
AlexanderYastrebov added a commit that referenced this pull request Jul 11, 2023
Use ./... instead of loop over packages.

Originally loop over packages was added by #359 that introduced glide package manager.
This manager fetches dependencies into vendor/ subdirectory (see https://glide.readthedocs.io/en/latest/getting-started/) and
apparently loop over packages was necessary back then.
glide usage was removed by #764

The change now also runs testable examples.

Also:
* increases redis testcontainer ping and shutdown timeouts after observed CI failures
* fixes cmd/eskip etcd shutdown similar to #2359
* fixes cmd/eskip test introduced by #2202
* fixes lua example
* removed bench target as it makes little sense
* .coverprofile-all can not be simplified yet due to golang/go#45361

Signed-off-by: Alexander Yastrebov <[email protected]>
MustafaSaber pushed a commit that referenced this pull request Jul 12, 2023
Use ./... instead of loop over packages.

Originally loop over packages was added by #359 that introduced glide package manager.
This manager fetches dependencies into vendor/ subdirectory (see https://glide.readthedocs.io/en/latest/getting-started/) and
apparently loop over packages was necessary back then.
glide usage was removed by #764

The change now also runs testable examples.

Also:
* increases redis testcontainer ping and shutdown timeouts after observed CI failures
* fixes cmd/eskip etcd shutdown similar to #2359
* fixes cmd/eskip test introduced by #2202
* fixes lua example
* removed bench target as it makes little sense
* .coverprofile-all can not be simplified yet due to golang/go#45361

Signed-off-by: Alexander Yastrebov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants