From ae4240788c5b1f2e2887a1de98174bb62ca4205a Mon Sep 17 00:00:00 2001 From: Andrew Kaster Date: Tue, 29 Jun 2021 22:52:41 -0600 Subject: [PATCH] Userland: Unlink file after waiting for child in run-tests TestProcFs expects to be able to stat its stdout and stderr. The new ProcFS implemetnation properly forwards the symlinks for /proc/pid/fd/[1,2] to the temporary file that we had unlinked prior to spawning the process. However, this makes it so that a normal stat on the symlink to that file fails (as expected). Move the unlink to after we've waited on the child, so that we know it won't be trying any funny business with its stdout/stderr anymore. --- Userland/Utilities/run-tests.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Userland/Utilities/run-tests.cpp b/Userland/Utilities/run-tests.cpp index 7cd3d20258..4ad6f77641 100644 --- a/Userland/Utilities/run-tests.cpp +++ b/Userland/Utilities/run-tests.cpp @@ -193,9 +193,6 @@ FileResult TestRunner::run_test_file(const String& test_path) int child_out_err_file = mkstemp(child_out_err_path); VERIFY(child_out_err_file >= 0); - int ret = unlink(child_out_err_path); - VERIFY(ret == 0); - (void)posix_spawn_file_actions_adddup2(&file_actions, child_out_err_file, STDOUT_FILENO); (void)posix_spawn_file_actions_adddup2(&file_actions, child_out_err_file, STDERR_FILENO); (void)posix_spawn_file_actions_addchdir(&file_actions, path_for_test.dirname().characters()); @@ -209,7 +206,7 @@ FileResult TestRunner::run_test_file(const String& test_path) pid_t child_pid = -1; // FIXME: Do we really want to copy test runner's entire env? - ret = posix_spawn(&child_pid, test_path.characters(), &file_actions, nullptr, const_cast(argv.data()), environ); + int ret = posix_spawn(&child_pid, test_path.characters(), &file_actions, nullptr, const_cast(argv.data()), environ); VERIFY(ret == 0); VERIFY(child_pid > 0); @@ -234,6 +231,12 @@ FileResult TestRunner::run_test_file(const String& test_path) kill(child_pid, SIGCONT); } } + + // Remove the child's stdout from /tmp. This does cause the temp file to be observable + // while the test is executing, but if it hangs that might even be a bonus :) + ret = unlink(child_out_err_path); + VERIFY(ret == 0); + return FileResult { move(path_for_test), get_time_in_ms() - start_time, test_result, child_out_err_file }; }