Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove test timers related to code block #380

Merged
merged 2 commits into from
Jul 24, 2024
Merged

Remove test timers related to code block #380

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
36c6752
Remove test timers related to code block
sejas Jul 23, 2024
cdb3e17
Go back to use runOnlyPendingTimersAsync on assistant code block
sejas Jul 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/components/tests/ai-clear-history-reminder.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ describe( 'AIClearHistoryReminder', () => {
window.HTMLElement.prototype.scrollIntoView = jest.fn();
clearInput = jest.fn();
jest.clearAllMocks();
jest.useFakeTimers();
jest.setSystemTime( MOCKED_TIME );
} );

Expand Down
19 changes: 12 additions & 7 deletions src/components/tests/assistant-code-block.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { render, screen, fireEvent, act } from '@testing-library/react';
import { act, render, screen, fireEvent } from '@testing-library/react';
import { getIpcApi } from '../../lib/get-ipc-api';
import createCodeComponent from '../assistant-code-block';

Expand Down Expand Up @@ -89,6 +89,14 @@ describe( 'createCodeComponent', () => {
} );

describe( 'when the "run" button is clicked', () => {
beforeEach( () => {
jest.useFakeTimers();
} );

afterEach( () => {
jest.useRealTimers();
} );

it( 'should display an activity indicator while running code', async () => {
( getIpcApi as jest.Mock ).mockReturnValue( {
executeWPCLiInline: jest.fn().mockResolvedValue( { stdout: 'Mock success', stderr: '' } ),
Expand All @@ -100,8 +108,7 @@ describe( 'createCodeComponent', () => {

expect( screen.getByText( 'Running...' ) ).toBeVisible();

// Run code execution measurement timer
await act( () => jest.runAllTimersAsync() );
await act( () => jest.runOnlyPendingTimersAsync() );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could keep this line if we use fake timers.

diff --git forkSrcPrefix/src/components/tests/assistant-code-block.test.tsx forkDstPrefix/src/components/tests/assistant-code-block.test.tsx
index 2cee11e4fbdacf4383f20ee731f97aaedaadd346..3da280e60c199a619857b02d20ce2d58f64bbfd3 100644
--- forkSrcPrefix/src/components/tests/assistant-code-block.test.tsx
+++ forkDstPrefix/src/components/tests/assistant-code-block.test.tsx
@@ -1,4 +1,4 @@
-import { render, screen, fireEvent, waitFor } from '@testing-library/react';
+import { act, render, screen, fireEvent, waitFor } from '@testing-library/react';
 import { getIpcApi } from '../../lib/get-ipc-api';
 import createCodeComponent from '../assistant-code-block';
 
@@ -89,6 +89,14 @@ describe( 'createCodeComponent', () => {
 	} );
 
 	describe( 'when the "run" button is clicked', () => {
+		beforeEach( () => {
+			jest.useFakeTimers();
+		} );
+
+		afterEach( () => {
+			jest.useRealTimers();
+		} );
+
 		it( 'should display an activity indicator while running code', async () => {
 			( getIpcApi as jest.Mock ).mockReturnValue( {
 				executeWPCLiInline: jest.fn().mockResolvedValue( { stdout: 'Mock success', stderr: '' } ),
@@ -100,9 +108,9 @@ describe( 'createCodeComponent', () => {
 
 			expect( screen.getByText( 'Running...' ) ).toBeVisible();
 
-			await waitFor( () => {
-				expect( screen.queryByText( 'Running...' ) ).not.toBeInTheDocument();
-			} );
+			await act( () => jest.runOnlyPendingTimersAsync() );
+
+			expect( screen.queryByText( 'Running...' ) ).not.toBeInTheDocument();
 		} );
 
 		it( 'should display the output of the successfully executed code', async () => {
@@ -113,10 +121,10 @@ describe( 'createCodeComponent', () => {
 
 			fireEvent.click( screen.getByText( 'Run' ) );
 
-			await waitFor( () => {
-				expect( screen.getByText( 'Success' ) ).toBeVisible();
-				expect( screen.getByText( 'Mock success' ) ).toBeVisible();
-			} );
+			await act( () => jest.runOnlyPendingTimersAsync() );
+
+			expect( screen.getByText( 'Success' ) ).toBeVisible();
+			expect( screen.getByText( 'Mock success' ) ).toBeVisible();
 		} );
 
 		it( 'should display the output of the failed code execution', async () => {
@@ -127,10 +135,10 @@ describe( 'createCodeComponent', () => {
 
 			fireEvent.click( screen.getByText( 'Run' ) );
 
-			await waitFor( () => {
-				expect( screen.getByText( 'Error' ) ).toBeVisible();
-				expect( screen.getByText( 'Mock error' ) ).toBeVisible();
-			} );
+			await act( () => jest.runOnlyPendingTimersAsync() );
+
+			expect( screen.getByText( 'Error' ) ).toBeVisible();
+			expect( screen.getByText( 'Mock error' ) ).toBeVisible();
 		} );
 	} );

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good! I reverted that change in favor of the timers cdb3e17


expect( screen.queryByText( 'Running...' ) ).not.toBeInTheDocument();
} );
Expand All @@ -114,8 +121,7 @@ describe( 'createCodeComponent', () => {

fireEvent.click( screen.getByText( 'Run' ) );

// Run code execution measurement timer
await act( () => jest.runAllTimersAsync() );
await act( () => jest.runOnlyPendingTimersAsync() );

expect( screen.getByText( 'Success' ) ).toBeVisible();
expect( screen.getByText( 'Mock success' ) ).toBeVisible();
Expand All @@ -129,8 +135,7 @@ describe( 'createCodeComponent', () => {

fireEvent.click( screen.getByText( 'Run' ) );

// Run code execution measurement timer
await act( () => jest.runAllTimersAsync() );
await act( () => jest.runOnlyPendingTimersAsync() );

expect( screen.getByText( 'Error' ) ).toBeVisible();
expect( screen.getByText( 'Mock error' ) ).toBeVisible();
Expand Down
4 changes: 2 additions & 2 deletions src/components/tests/content-tab-assistant.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ describe( 'ContentTabAssistant', () => {
const storageKey = 'ai_chat_messages';
localStorage.setItem( storageKey, JSON.stringify( { [ runningSite.id ]: initialMessages } ) );
render( <ContentTabAssistant selectedSite={ runningSite } /> );
jest.advanceTimersByTime( MIMIC_CONVERSATION_DELAY + 1000 );
await waitFor( () => {
expect( screen.getByText( 'Initial message 1' ) ).toBeVisible();
expect( screen.getByText( 'Initial message 2' ) ).toBeVisible();
Expand Down Expand Up @@ -214,7 +213,6 @@ describe( 'ContentTabAssistant', () => {
act( () => {
fireEvent.change( textInput, { target: { value: 'New message' } } );
fireEvent.keyDown( textInput, { key: 'Enter', code: 'Enter' } );
jest.advanceTimersByTime( MIMIC_CONVERSATION_DELAY + 1000 );
} );
await waitFor( () => {
expect( screen.getByText( 'New message' ) ).toBeVisible();
Expand Down Expand Up @@ -325,6 +323,7 @@ describe( 'ContentTabAssistant', () => {
test( 'clears history via reminder when last message is two hours old', async () => {
const MOCKED_TIME = 1718882159928;
const TWO_HOURS_DIFF = 2 * 60 * 60 * 1000;
jest.useFakeTimers();
jest.setSystemTime( MOCKED_TIME );

const storageKey = 'ai_chat_messages';
Expand Down Expand Up @@ -369,6 +368,7 @@ describe( 'ContentTabAssistant', () => {
},
{ timeout: MIMIC_CONVERSATION_DELAY + 1000 }
);
jest.useRealTimers();
} );

test( 'renders notices by importance', async () => {
Expand Down
Loading