Tests for process tree structure #64
Conversation
JayH5
commented
Jul 21, 2017
JayH5
commented
- Check the parent PIDs for things
- Sort processes into master/worker
example/test.py
Outdated
| for row in ps_rows: | ||
| ppid_to_rows.setdefault(row.ppid, []).append(row) | ||
|
|
||
| # There should only be to ppids: the parent of the master and the master |
There was a problem hiding this comment.
Sadly, it wouldn't be sufficiently accurate to write "Always two ppids there are; no more, no less. A master and an apprentice."
There was a problem hiding this comment.
Yeah :) I do want a test with multiple Gunicorn "apprentices" at some point.
example/test.py
Outdated
|
|
||
| assert_tini_pid_1(ps_data.pop(0), 'mysite.wsgi:application') | ||
| assert_that(ps_data, HasLength(5)) |
There was a problem hiding this comment.
Why did we need to bring this back?
There was a problem hiding this comment.
😿 because there isn't anything checking for stray, unknown processes now.
There was a problem hiding this comment.
Maybe we should remove things from the list as we assert on them and then assert that the list is empty at the end?
example/test.py
Outdated
| nginx_master, nginx_workers = find_prefork_worker_split(nginx_rows) | ||
| assert_that(nginx_master, matches_attributes_values( | ||
| ('ppid', 'ruser', 'args'), | ||
| # FIXME: Nginx should not be parented by Gunicorn |
There was a problem hiding this comment.
I'm guessing this is because we exec the gunicorn process at the end of our bootstrap script? In that case, everything we run in there will be parented by it.
There was a problem hiding this comment.
Yeah. This is a thing I've known about for a long time, but for the first time it's being tested \o/. I have a branch that sends a SIGKILL to the nginx master process.. and of course the container carries on running happily.
example/test.py
Outdated
|
|
||
| # There should only be two ppids: the parent of the master and the master | ||
| assert len(ppid_to_rows) == 2 |
There was a problem hiding this comment.
Assertions like this aren't very helpful for debugging, because all we know from the failure is that we had a different number of entries from what we're expecting.
|
|
||
|
|
||
| class MatchesPsTree(object): | ||
| def __init__(self, ruser, args, ppid=None, pid=None, children=()): |
There was a problem hiding this comment.
Does it still make sense to have ppid here?
There was a problem hiding this comment.
For completeness, I think it does. There's no reason not to have it, and people might want to examine subtrees and assert that the subtree's root has the right ppid.
|
👍 from me on @jerith's changes :) |
