From 3afe2d30480fc7c15a459dca67ecee9246e98c36 Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Sat, 6 Jul 2024 14:08:42 +0200 Subject: [PATCH] Fix orphaned browser processes due to uncaught exceptions in the tests If uncaught exceptions occur in the tests (which happened in #17962 and can be triggered manually by throwing an error in `integration-boot.js`) the teardown logic of the tests doesn't get to run and thus spawned browser processes are not closed properly. Given that `test.mjs` is the only process that has a reference to them they will become orphaned and keep running if `test.mjs` exits without explicitly closing them. This commit fixes the issue by always closing the browsers if uncaught exceptions occur, and we make sure to log them for debugging purposes. --- test/test.mjs | 40 +++++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/test/test.mjs b/test/test.mjs index 69a7959b5..b3315f30c 100644 --- a/test/test.mjs +++ b/test/test.mjs @@ -970,8 +970,6 @@ async function startBrowsers({ baseUrl, initializeSession }) { await puppeteer.trimCache(); const browserNames = options.noChrome ? ["firefox"] : ["firefox", "chrome"]; - - sessions = []; for (const browserName of browserNames) { // The session must be pushed first and augmented with the browser once // it's initialized. The reason for this is that browser initialization @@ -1078,25 +1076,33 @@ async function main() { stats = []; } - if (options.downloadOnly) { - await ensurePDFsDownloaded(); - } else if (options.unitTest) { - // Allows linked PDF files in unit-tests as well. - await ensurePDFsDownloaded(); - startUnitTest("/test/unit/unit_test.html", "unit"); - } else if (options.fontTest) { - startUnitTest("/test/font/font_test.html", "font"); - } else if (options.integration) { - // Allows linked PDF files in integration-tests as well. - await ensurePDFsDownloaded(); - startIntegrationTest(); - } else { - startRefTest(options.masterMode, options.reftest); + try { + if (options.downloadOnly) { + await ensurePDFsDownloaded(); + } else if (options.unitTest) { + // Allows linked PDF files in unit-tests as well. + await ensurePDFsDownloaded(); + await startUnitTest("/test/unit/unit_test.html", "unit"); + } else if (options.fontTest) { + await startUnitTest("/test/font/font_test.html", "font"); + } else if (options.integration) { + // Allows linked PDF files in integration-tests as well. + await ensurePDFsDownloaded(); + await startIntegrationTest(); + } else { + await startRefTest(options.masterMode, options.reftest); + } + } catch (e) { + // Close the browsers if uncaught exceptions occur, otherwise the spawned + // processes can become orphaned and keep running after `test.mjs` exits + // because the teardown logic of the tests did not get a chance to run. + console.error(e); + await Promise.all(sessions.map(session => closeSession(session.name))); } } var server; -var sessions; +var sessions = []; var onAllSessionsClosed; var host = "127.0.0.1"; var options = parseOptions();