Add test to ensure all profiler-spawned processes go down if gProfiler is brutally killed#780
Add test to ensure all profiler-spawned processes go down if gProfiler is brutally killed#780mpozniak95 wants to merge 4 commits intointel:masterfrom
Conversation
| profiler_flags: List[str], | ||
| application_docker_mount: bool, | ||
| ) -> None: | ||
| """Tests if killing gprofiler with -9 results in killing py-spy""" |
There was a problem hiding this comment.
I'd give a link to the issue #674 here which explains a bit more about the reasoning.
| @pytest.mark.parametrize("in_container", [True]) | ||
| @pytest.mark.parametrize("application_docker_mount", [True]) | ||
| @pytest.mark.parametrize("application_docker_capabilities", ["SYS_PTRACE"]) | ||
| def test_killing_spawned_processes( |
There was a problem hiding this comment.
Note that test_executable.py runs in a matrix for different images, while this one we actually need only one time (like test_sanity.py f.e).
The problem is, the workflow of executable tests has the exe, and the workflow of the other tests (like test_sanity.py) don't have it. We can do the same testing with the container version, but it's more work.
I think, since we're planning on doing #582 soon, it makes more sense to merge it nad then it'll be easier to write this test.
| command = ["ls", "/tmp/gprofiler_tmp"] | ||
| e, ls_output = application_docker_container.exec_run(cmd=command, privileged=True, user="root:root", detach=False) | ||
| assert "tmp" in ls_output.decode() |
There was a problem hiding this comment.
This means to ensure that part?
The test should ensure that gProfiler was indeed brutally killed - i.e, no cleanups were performed, the process exited with SIGKILL exit code, the temporary directory at /tmp/gprofiler_tmp/tmpxxxxx remains on the filesystem.
Please add a comment, the purpose isn't clear. Also the test can be made more robust, this is not enough - even if the directory is missing, output will be ls: cannot access '/tmp/....': no such file... and your assert still passes, won't it?
Should check either for the exact directory /tmp/gprofiler_tmp/tmpxxxx OR for the unpacked staticx executable (which is removed after gprofiler exits gracefully, but not when terminated). I think checking the directory is nicer, but the exact one. You can find it by running ls earlier and finding it.
|
Marking this draft until #790 is merged. |
|
With #790 merged, this can be revived @mpozniak95 if you have the time for it. |
2 participants
Description
Test checks if py-spy will go down if gprofiler processes are brutally killed.
Related Issue
#674