On Mon Jul 7, 2025 at 11:35 CEST, Phillip Wood wrote: > On 07/07/2025 10:02, Patrick Steinhardt wrote: >> I think this exported environment variable now leaks into subsequent >> tests, doesn't it? We may want to do it in a subshell. > > That's a very good point. Sure thing, I took inspiration from the previous ssh-agent test and it exports the ssh-agent variables without spawning a subshell, so I figured it was fine, but it was only because test_when_finished will fail in a subshell. I guess it's probably fine to leak only the ssh-agent variables accross tests and keep TMPDIR in a subshell. > The idiomatic way to test for an empty file is "test_file_is_empty > <file>" which avoids spawning wc. Thanks, I will update this. > As this test is an abridged version of the other test that uses > ssh-agent I think it would be more efficient to tweak that test to check > there are no temporary files left over. Our test suite is slow and > tweaking an existing test to improve our coverage instead of adding a > new test means we don't make it any slower than necessary. I moved this test out of the other ssh-agent one according to the advice from brian on the previous version. I agree that spawning multiple ssh-agent is definitly not the best for speed, but is it worth it to make the test failure less clear? I feel like it's not the best to keep checks for temporary files clean up in a test that (initially) only claims to sign commits. -- redoste
