https://pantsbuild.org/ logo
Join Slack
Powered by
How do I ensure that pytest fixtures are correctly...
# general
m
How do I ensure that pytest fixtures are correctly cleaned up when the tests are restarted due to a filesystem change? For more background, I start a docker container in a fixture (database) and under normal circumstances, it gets closed. But when I'm running tests that restart due to file system changes, the usual signal handling doesn't work, and I often end up with dozens of docker containers when running tests while editing.
h
I don't think there is currently a way - Pants is hard-killing the pytest process, so the internal fixture cleanups have no chance to run. There has been past discussion of supporting setup/teardown hooks at the pants level, but it has not been implemented.
An alternative would be to support running tests without hard interruption
You can run with 
--no-watch-filesystem
, but then you also have to set 
--no-pantsd
, which will hurt performance
m
Thanks for the reply, even if the answer is 'no'. 😕 That discussion definitely is related, though it's a bit more abstract than the feature I'm looking for, which has a number of other alternative approaches that may be less "heavy-handed" than complete customization of the test rule (which is probably beyond the effort I would put in anyway). For example, killing the process with a SIGTERM would work for 
pytest
, though I don't know whether this is possible. Is github the appropriate place to make a feature request like this?
h
I wonder why we do that. SIGINT implies "interrupt the process", not "terminate the process", although many processes treat an interruption as a termination.
But I am sure there is history there
@witty-crayon-22786 any idea why we send SIGINT and not SIGTERM in the process shutdown sequence?
w
We moved from SIGTERM to SIGINT to allow for graceful shutdown. But we do eventually send SIGTERM, iirc?
h
We send SIGINT and then SIGKILL
I think we should be sending SIGTERM and then SIGKILL
w
...but, hm. It sounds like Michael is asking for SIGINT
h
SIGTERM is "shut this process down gracefully"
SIGINT is "interrupt this process" which is not the same thing
I think Michael is asking for SIGTERM
w
In any case, I think that right now the only thing that is shut down gracefully is foreground/InteractiveProcess, rather than tests
m
I was asking for one of them. I'm surprised to see that it sends SIGINT, so I'm looking into why I'm not seeing it. Possibilities: not configured to run graceful shutdown, bug in my testing code.
w
Yea, I think that that is right: I don't think that we gracefully kill test processes currently. Only 
InteractiveProcess
👍 1
m
A graceful shutdown is almost surely sufficient to allow me to clean up my fixtures. Cf. pytest docs here.
w
There are multiple places where we should likely (optionally?) be doing graceful shutdown for 
Process
h
Aha
yeah, we likely want to gracefully shutdown all tool runs
certainly tests
m
Since it's the holiday season, it would definitely help if I could also configure a timeout before SIGKILL during graceful shutdown. 🎅 🎁
w
Can use the same helper that we use for InteractiveProcess.
It's not configurable in that case (currently), but could be in this case.
m
(I jest; seriously, thanks for the help.)
The use case for the timeout: killing docker containers may take some actual wall-clock time, and python can be slow in general.
w
Well, don't thank us too soon. Still need to find someone to actually work on it =p
😅 1
m
Honestly, just knowing that it can't be done right now saves time for me.
h
A comment on that ticket with the pytest specifics would be great
As well as any legwork you've done to see if SIGINT is sufficient or would it need SIGTERM
👍 1
m
Will do. Preference for commenting on #16878 vs. #17860? Both are somewhat related.
I'll make a new one and link it. 🙂
w
Please comment on the existing one and I can retitle it.
👍 1
m
Will do. Presumably #17860.
w
Sorry: the other one
✅ 1
✅ 1
Graceful shutdown is probably something transparently on by default so I don't really view it as a "customization"
m
Commented. Thanks!
🙏 1
h
I've assigned to myself as a fun little rust endeavor
❤️ 1
w
Thanks Benjy: feel free to ping if you have questions.
2 Views
Open in Slack
PreviousNext
Powered by