Skip to content

Commit 1cb87ca

Browse files
committed
🔧 Address PR feedback: fix async cleanup race conditions
- Await cleanup() in all event handlers to ensure proper resource release - Remove unused cleanupReason variable - Add stack trace to main catch block for consistency
1 parent b7ba620 commit 1cb87ca

File tree

2 files changed

+15
-16
lines changed

2 files changed

+15
-16
lines changed

clients/ember/bin/vizzly-testem-launcher.js

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ if (existsSync(configPath)) {
5858
let browserInstance = null;
5959
let screenshotServer = null;
6060
let isShuttingDown = false;
61-
let cleanupReason = 'unknown';
6261

6362
/**
6463
* Clean up resources and exit
@@ -68,7 +67,6 @@ let cleanupReason = 'unknown';
6867
async function cleanup(reason = 'unknown', exitCode = 0) {
6968
if (isShuttingDown) return;
7069
isShuttingDown = true;
71-
cleanupReason = reason;
7270

7371
if (process.env.VIZZLY_LOG_LEVEL === 'debug') {
7472
console.error(`[vizzly-testem-launcher] Cleanup triggered: ${reason}`);
@@ -120,22 +118,23 @@ async function main() {
120118
playwrightOptions,
121119
onPageCreated: page => {
122120
setPage(page);
123-
page.on('close', () => cleanup('page-close'));
121+
page.on('close', async () => await cleanup('page-close'));
124122
},
125-
onBrowserDisconnected: () => cleanup('browser-disconnected'),
123+
onBrowserDisconnected: async () => await cleanup('browser-disconnected'),
126124
});
127125

128126
// 4. Listen for browser crashes
129127
let { page } = browserInstance;
130-
page.on('crash', () => {
128+
page.on('crash', async () => {
131129
console.error('[vizzly-testem-launcher] Page crashed!');
132-
cleanup('page-crash', 1);
130+
await cleanup('page-crash', 1);
133131
});
134132

135133
// 5. Keep process alive until cleanup is called
136134
await new Promise(() => {});
137135
} catch (error) {
138136
console.error('[vizzly-testem-launcher] Failed to start:', error.message);
137+
console.error(error.stack);
139138

140139
if (screenshotServer) {
141140
await stopScreenshotServer(screenshotServer).catch(() => {});
@@ -146,24 +145,24 @@ async function main() {
146145
}
147146

148147
// Handle graceful shutdown signals from Testem
149-
process.on('SIGTERM', () => cleanup('SIGTERM'));
150-
process.on('SIGINT', () => cleanup('SIGINT'));
151-
process.on('SIGHUP', () => cleanup('SIGHUP'));
148+
process.on('SIGTERM', async () => await cleanup('SIGTERM'));
149+
process.on('SIGINT', async () => await cleanup('SIGINT'));
150+
process.on('SIGHUP', async () => await cleanup('SIGHUP'));
152151

153152
// Handle unexpected errors
154-
process.on('uncaughtException', error => {
153+
process.on('uncaughtException', async error => {
155154
console.error('[vizzly-testem-launcher] Uncaught exception:', error.message);
156155
console.error(error.stack);
157-
cleanup('uncaughtException', 1);
156+
await cleanup('uncaughtException', 1);
158157
});
159158

160-
process.on('unhandledRejection', (reason, promise) => {
159+
process.on('unhandledRejection', async (reason, promise) => {
161160
console.error('[vizzly-testem-launcher] Unhandled rejection at:', promise);
162161
console.error('[vizzly-testem-launcher] Reason:', reason);
163162
if (reason instanceof Error) {
164163
console.error(reason.stack);
165164
}
166-
cleanup('unhandledRejection', 1);
165+
await cleanup('unhandledRejection', 1);
167166
});
168167

169168
main();

clients/ember/test-app/tests/integration/components/alert-test.gjs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { module, test } from 'qunit';
22
import { setupRenderingTest } from 'ember-qunit';
33
import { render } from '@ember/test-helpers';
44
import Alert from 'test-ember-app/components/alert';
5-
import { vizzlySnapshot } from '@vizzly-testing/ember/test-support';
5+
import { vizzlyScreenshot } from '@vizzly-testing/ember/test-support';
66

77
module('Integration | Component | Alert', function (hooks) {
88
setupRenderingTest(hooks);
@@ -33,7 +33,7 @@ module('Integration | Component | Alert', function (hooks) {
3333
assert.dom('[data-test-alert="warning"]').exists();
3434
assert.dom('[data-test-alert="error"]').exists();
3535

36-
await vizzlySnapshot('alert-variants');
36+
await vizzlyScreenshot('alert-variants');
3737
});
3838

3939
test('it renders without title', async function (assert) {
@@ -48,6 +48,6 @@ module('Integration | Component | Alert', function (hooks) {
4848
assert.dom('[data-test-alert="no-title"]').exists();
4949
assert.dom('[data-test-alert="no-title"] .alert-title').doesNotExist();
5050

51-
await vizzlySnapshot('alert-no-title');
51+
await vizzlyScreenshot('alert-no-title');
5252
});
5353
});

0 commit comments

Comments
 (0)