Skip to content

Commit 681b8d5

Browse files
committed
test: strengthen weak tests with proper assertions
- Multiple assets test: now verifies correct asset selection - NODE_ENV defaults: split into 2 tests (dev/prod) with real assertions - Worker without pid: explicit no-throw and no-hang assertions - Add production NODE_ENV test for keyboard default
1 parent 9240579 commit 681b8d5

1 file changed

Lines changed: 65 additions & 27 deletions

File tree

__tests__/RunScriptWebpackPlugin.test.ts

Lines changed: 65 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -381,9 +381,20 @@ describe('RunScriptWebpackPlugin', () => {
381381
const doneCallback = jest.fn();
382382
callback(compilation, doneCallback);
383383

384+
// Verify warning is logged so users know which entry was auto-selected
384385
expect(consoleLogSpy).toHaveBeenCalledWith(
385386
expect.stringContaining('More than one entry built')
386387
);
388+
expect(consoleLogSpy).toHaveBeenCalledWith(
389+
expect.stringContaining('main.js')
390+
);
391+
392+
// Verify the first asset is correctly selected and executed
393+
expect(mockFork).toHaveBeenCalledWith(
394+
'/dist/main.js',
395+
[],
396+
expect.anything()
397+
);
387398
});
388399

389400
describe('restartable option with keyboard input', () => {
@@ -501,21 +512,16 @@ describe('RunScriptWebpackPlugin', () => {
501512
});
502513
});
503514

504-
describe('keyboard option defaults', () => {
515+
describe('keyboard option defaults based on NODE_ENV', () => {
505516
const originalNodeEnv = process.env.NODE_ENV;
517+
let stdinOnSpy: jest.Mock;
518+
let stdinSetEncodingSpy: jest.Mock;
519+
let originalStdin: typeof process.stdin;
506520

507-
afterEach(() => {
508-
process.env.NODE_ENV = originalNodeEnv;
509-
});
510-
511-
it('should default keyboard to true when NODE_ENV is development', () => {
512-
process.env.NODE_ENV = 'development';
513-
514-
// We need to re-import to pick up the new NODE_ENV
515-
// Instead, we test that restartable triggers keyboard behavior when NODE_ENV is development
516-
const stdinOnSpy = jest.fn();
517-
const stdinSetEncodingSpy = jest.fn();
518-
const originalStdin = process.stdin;
521+
beforeEach(() => {
522+
stdinOnSpy = jest.fn();
523+
stdinSetEncodingSpy = jest.fn();
524+
originalStdin = process.stdin;
519525

520526
Object.defineProperty(process, 'stdin', {
521527
value: {
@@ -525,28 +531,59 @@ describe('RunScriptWebpackPlugin', () => {
525531
configurable: true,
526532
});
527533

528-
// Create plugin with restartable but NOT specifying keyboard
529-
// In development, keyboard should default to true
530-
const { RunScriptWebpackPlugin: FreshPlugin } = jest.requireActual('../src/index') as { RunScriptWebpackPlugin: typeof RunScriptWebpackPlugin };
531-
532-
// Since we can't easily re-evaluate the default, we test the documented behavior
533-
// The keyboard option defaults based on NODE_ENV at instantiation time
534+
// Reset module cache to pick up new NODE_ENV
535+
jest.resetModules();
536+
});
534537

538+
afterEach(() => {
539+
process.env.NODE_ENV = originalNodeEnv;
535540
Object.defineProperty(process, 'stdin', {
536541
value: originalStdin,
537542
configurable: true,
538543
});
539544
});
545+
546+
it('should enable keyboard by default when NODE_ENV is development and restartable is true', () => {
547+
// This tests the documented behavior: keyboard defaults to true in development
548+
// This is important because users expect 'rs' to work without explicit config
549+
process.env.NODE_ENV = 'development';
550+
551+
// Fresh import with new NODE_ENV
552+
const { RunScriptWebpackPlugin: FreshPlugin } = require('../src/index');
553+
554+
const plugin = new FreshPlugin({ restartable: true });
555+
plugin.apply(compiler);
556+
557+
expect(stdinSetEncodingSpy).toHaveBeenCalledWith('utf8');
558+
expect(stdinOnSpy).toHaveBeenCalledWith('data', expect.any(Function));
559+
});
560+
561+
it('should NOT enable keyboard by default when NODE_ENV is production', () => {
562+
// In production, keyboard should default to false to prevent
563+
// the server from hanging on stdin, which would cause issues in containers
564+
process.env.NODE_ENV = 'production';
565+
566+
const { RunScriptWebpackPlugin: FreshPlugin } = require('../src/index');
567+
568+
const plugin = new FreshPlugin({ restartable: true });
569+
plugin.apply(compiler);
570+
571+
expect(stdinSetEncodingSpy).not.toHaveBeenCalled();
572+
expect(stdinOnSpy).not.toHaveBeenCalled();
573+
});
540574
});
541575

542-
it('should handle restart when worker has no pid', () => {
576+
it('should gracefully handle restart when worker process has no pid (defensive edge case)', () => {
577+
// This tests graceful degradation when the forked process doesn't have a pid.
578+
// While rare, this can happen if the process exits immediately or in mocked environments.
579+
// The plugin should NOT throw and should continue working.
543580
const plugin = new RunScriptWebpackPlugin({ name: 'main.js' });
544581
plugin.apply(compiler);
545582

546583
const tapAsyncMock = compiler.hooks.afterEmit.tapAsync as jest.Mock;
547584
const callback = tapAsyncMock.mock.calls[0][1];
548585

549-
// Mock fork to return a worker without pid
586+
// Simulate a worker that was created but has undefined pid
550587
mockFork.mockReturnValue({
551588
pid: undefined,
552589
connected: true,
@@ -556,23 +593,24 @@ describe('RunScriptWebpackPlugin', () => {
556593

557594
const doneCallback = jest.fn();
558595

559-
// First run
596+
// First run - starts server with undefined pid
560597
callback(compilation, doneCallback);
561598
jest.runAllTimers();
599+
expect(doneCallback).toHaveBeenCalledTimes(1);
562600

563-
// Mock now returns with pid after first start
601+
// Mock now returns healthy worker
564602
mockFork.mockReturnValue({
565603
pid: 456,
566604
connected: true,
567605
kill: jest.fn(),
568606
on: jest.fn()
569607
});
570608

571-
// Second run - should try to restart but worker.pid was undefined
572-
callback(compilation, doneCallback);
609+
// Second run - should not crash when trying to kill worker with undefined pid
610+
expect(() => callback(compilation, doneCallback)).not.toThrow();
611+
expect(process.kill).not.toHaveBeenCalled(); // Can't kill undefined pid
573612

574-
// process.kill should not be called because pid was undefined
575-
expect(process.kill).not.toHaveBeenCalled();
576613
jest.runAllTimers();
614+
expect(doneCallback).toHaveBeenCalledTimes(2); // Callback still called - no hang
577615
});
578616
});

0 commit comments

Comments
 (0)