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

test: fix process leak in wait e2e test #10790

Merged
merged 1 commit into from Jul 10, 2023
Merged

Conversation

milas
Copy link
Member

@milas milas commented Jul 10, 2023

What I did

  • Run down before and after test to not leave around containers
  • Kill the wait process that's waiting on infinity
    • NOTE: If the test is actually working, this should exit once the down happens, but this ensures that we kill everything we start

I'd like to generalize more of this into the framework, but this is a quick fix to prevent filling up CI machines with tons of processes over time.

(not mandatory) A picture of a cute animal, if possible in relation to what you did
two rabbits coming out of a burrow

* Run `down` before and after test to not leave around containers
* Kill the `wait` process that's waiting on `infinity`
  * NOTE: If the test is actually working, this should exit once
    the `down` happens, but this ensures that we kill everything
    we start

I'd like to generalize more of this into the framework, but this
is a quick fix to prevent filling up CI machines with tons of
processes over time.

Signed-off-by: Milas Bowman <milas.bowman@docker.com>
@milas milas requested a review from a team July 10, 2023 12:43
@milas milas self-assigned this Jul 10, 2023
@milas milas requested review from nicksieger, ndeloof, StefanScherer, ulyssessouza, glours and laurazard and removed request for a team July 10, 2023 12:43
Copy link
Contributor

@glours glours left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@codecov
Copy link

codecov bot commented Jul 10, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.01 🎉

Comparison is base (bc6ad2e) 59.11% compared to head (8dea7b5) 59.12%.

Additional details and impacted files
@@            Coverage Diff             @@
##               v2   #10790      +/-   ##
==========================================
+ Coverage   59.11%   59.12%   +0.01%     
==========================================
  Files         115      115              
  Lines        9853     9853              
==========================================
+ Hits         5825     5826       +1     
+ Misses       3437     3436       -1     
  Partials      591      591              

see 5 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@glours glours merged commit 7a9dfa4 into docker:v2 Jul 10, 2023
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants