From 32d03347fc50796477b732adb53a125257881fa3 Mon Sep 17 00:00:00 2001 From: LordMathis Date: Sun, 29 Jun 2025 16:06:35 +0200 Subject: [PATCH] Refactor modal tests for improved clarity and consistency in content and actions --- .../account/DeleteAccountModal.test.tsx | 48 +++++++++++++------ .../modals/account/DeleteAccountModal.tsx | 4 +- .../account/EmailPasswordModal.test.tsx | 8 ++-- .../modals/file/CreateFileModal.test.tsx | 38 +++++++++------ .../modals/file/CreateFileModal.tsx | 4 +- .../modals/file/DeleteFileModal.test.tsx | 32 ++++++++----- .../modals/file/DeleteFileModal.tsx | 4 +- .../modals/git/CommitMessageModal.test.tsx | 42 +++++++++------- .../modals/git/CommitMessageModal.tsx | 4 +- .../modals/user/CreateUserModal.test.tsx | 4 +- .../modals/user/EditUserModal.test.tsx | 4 +- .../workspace/CreateWorkspaceModal.test.tsx | 10 ++-- 12 files changed, 122 insertions(+), 80 deletions(-) diff --git a/app/src/components/modals/account/DeleteAccountModal.test.tsx b/app/src/components/modals/account/DeleteAccountModal.test.tsx index 1e79d13..104c5bb 100644 --- a/app/src/components/modals/account/DeleteAccountModal.test.tsx +++ b/app/src/components/modals/account/DeleteAccountModal.test.tsx @@ -28,8 +28,8 @@ describe('DeleteAccountModal', () => { mockOnConfirm.mockResolvedValue(undefined); }); - describe('Modal Visibility', () => { - it('shows modal with warning and form when opened', () => { + describe('Modal Visibility and Content', () => { + it('renders modal with correct content when opened', () => { render( { expect( screen.getByTestId('delete-account-password-input') ).toBeInTheDocument(); - expect(screen.getByTestId('cancel-delete-button')).toBeInTheDocument(); - expect(screen.getByTestId('confirm-delete-button')).toBeInTheDocument(); + expect( + screen.getByTestId('cancel-delete-account-button') + ).toBeInTheDocument(); + expect( + screen.getByTestId('confirm-delete-account-button') + ).toBeInTheDocument(); }); - it('hides modal when closed', () => { + it('does not render when closed', () => { render( { }); }); - describe('Password Input and Validation', () => { + describe('Form Validation', () => { it('updates password value when user types', () => { render( { ); const passwordInput = screen.getByTestId('delete-account-password-input'); - const deleteButton = screen.getByTestId('confirm-delete-button'); + const deleteButton = screen.getByTestId('confirm-delete-account-button'); // Test empty password fireEvent.click(deleteButton); @@ -104,8 +108,10 @@ describe('DeleteAccountModal', () => { fireEvent.click(deleteButton); expect(mockOnConfirm).not.toHaveBeenCalled(); }); + }); - it('submits with valid password and clears field on success', async () => { + describe('User Actions', () => { + it('calls onConfirm with valid password and clears field on success', async () => { render( { ); const passwordInput = screen.getByTestId('delete-account-password-input'); - const deleteButton = screen.getByTestId('confirm-delete-button'); + const deleteButton = screen.getByTestId('confirm-delete-account-button'); fireEvent.change(passwordInput, { target: { value: 'validpassword' } }); fireEvent.click(deleteButton); @@ -129,6 +135,20 @@ describe('DeleteAccountModal', () => { }); }); + it('calls onClose when cancel button is clicked', () => { + render( + + ); + + fireEvent.click(screen.getByTestId('cancel-delete-account-button')); + expect(mockOnClose).toHaveBeenCalled(); + expect(mockOnConfirm).not.toHaveBeenCalled(); + }); + it('preserves password in field when submission fails', async () => { mockOnConfirm.mockRejectedValue(new Error('Invalid password')); @@ -141,7 +161,7 @@ describe('DeleteAccountModal', () => { ); const passwordInput = screen.getByTestId('delete-account-password-input'); - const deleteButton = screen.getByTestId('confirm-delete-button'); + const deleteButton = screen.getByTestId('confirm-delete-account-button'); fireEvent.change(passwordInput, { target: { value: 'wrongpassword' } }); fireEvent.click(deleteButton); @@ -165,7 +185,7 @@ describe('DeleteAccountModal', () => { /> ); - const cancelButton = screen.getByTestId('cancel-delete-button'); + const cancelButton = screen.getByTestId('cancel-delete-account-button'); fireEvent.click(cancelButton); expect(mockOnClose).toHaveBeenCalled(); @@ -182,7 +202,7 @@ describe('DeleteAccountModal', () => { ); const passwordInput = screen.getByTestId('delete-account-password-input'); - const deleteButton = screen.getByTestId('confirm-delete-button'); + const deleteButton = screen.getByTestId('confirm-delete-account-button'); fireEvent.change(passwordInput, { target: { value: 'testpassword' } }); @@ -240,7 +260,7 @@ describe('DeleteAccountModal', () => { fireEvent.change(passwordInput, { target: { value: 'userpassword' } }); // 3. User confirms deletion - const deleteButton = screen.getByTestId('confirm-delete-button'); + const deleteButton = screen.getByTestId('confirm-delete-account-button'); fireEvent.click(deleteButton); // 4. System processes deletion @@ -267,7 +287,7 @@ describe('DeleteAccountModal', () => { const passwordInput = screen.getByTestId('delete-account-password-input'); fireEvent.change(passwordInput, { target: { value: 'somepassword' } }); - const cancelButton = screen.getByTestId('cancel-delete-button'); + const cancelButton = screen.getByTestId('cancel-delete-account-button'); fireEvent.click(cancelButton); // Modal closes without deletion diff --git a/app/src/components/modals/account/DeleteAccountModal.tsx b/app/src/components/modals/account/DeleteAccountModal.tsx index 668704d..921e251 100644 --- a/app/src/components/modals/account/DeleteAccountModal.tsx +++ b/app/src/components/modals/account/DeleteAccountModal.tsx @@ -62,14 +62,14 @@ const DeleteAccountModal: React.FC = ({ diff --git a/app/src/components/modals/account/EmailPasswordModal.test.tsx b/app/src/components/modals/account/EmailPasswordModal.test.tsx index 3b6044a..e1a25fd 100644 --- a/app/src/components/modals/account/EmailPasswordModal.test.tsx +++ b/app/src/components/modals/account/EmailPasswordModal.test.tsx @@ -29,8 +29,8 @@ describe('EmailPasswordModal', () => { mockOnConfirm.mockResolvedValue(true); }); - describe('Modal Visibility', () => { - it('shows modal with email confirmation message when opened', () => { + describe('Modal Visibility and Content', () => { + it('renders modal with correct content when opened', () => { render( { ).toBeInTheDocument(); }); - it('hides modal when closed', () => { + it('does not render when closed', () => { render( { expect(screen.queryByText('Confirm Password')).not.toBeInTheDocument(); }); - it('displays different email addresses correctly', () => { + it('displays various email addresses correctly', () => { const customEmail = 'user@custom.com'; render( { mockModalContext.setNewFileModalVisible.mockClear(); }); - describe('Basic functionality', () => { - it('renders modal with all essential elements', () => { + describe('Modal Visibility and Content', () => { + it('renders modal with correct content when opened', () => { render(); expect(screen.getByText('Create New File')).toBeInTheDocument(); expect(screen.getByTestId('file-name-input')).toBeInTheDocument(); - expect(screen.getByTestId('cancel-create-button')).toBeInTheDocument(); - expect(screen.getByTestId('confirm-create-button')).toBeInTheDocument(); + expect( + screen.getByTestId('cancel-create-file-button') + ).toBeInTheDocument(); + expect( + screen.getByTestId('confirm-create-file-button') + ).toBeInTheDocument(); }); + }); - it('closes modal when cancel button is clicked', () => { + describe('User Actions', () => { + it('calls onClose when cancel button is clicked', () => { render(); - fireEvent.click(screen.getByTestId('cancel-create-button')); + fireEvent.click(screen.getByTestId('cancel-create-file-button')); expect(mockModalContext.setNewFileModalVisible).toHaveBeenCalledWith( false @@ -77,11 +83,13 @@ describe('CreateFileModal', () => { expect((fileNameInput as HTMLInputElement).value).toBe('test-file.md'); }); + }); + describe('Form Validation', () => { it('has disabled create button when input is empty', () => { render(); - const createButton = screen.getByTestId('confirm-create-button'); + const createButton = screen.getByTestId('confirm-create-file-button'); expect(createButton).toBeDisabled(); }); @@ -89,7 +97,7 @@ describe('CreateFileModal', () => { render(); const fileNameInput = screen.getByTestId('file-name-input'); - const createButton = screen.getByTestId('confirm-create-button'); + const createButton = screen.getByTestId('confirm-create-file-button'); fireEvent.change(fileNameInput, { target: { value: 'test.md' } }); @@ -97,12 +105,12 @@ describe('CreateFileModal', () => { }); }); - describe('File creation flow', () => { - it('creates file successfully with valid input', async () => { + describe('File Creation Flow', () => { + it('calls onCreateFile when confirmed with valid input', async () => { render(); const fileNameInput = screen.getByTestId('file-name-input'); - const createButton = screen.getByTestId('confirm-create-button'); + const createButton = screen.getByTestId('confirm-create-file-button'); fireEvent.change(fileNameInput, { target: { value: 'new-document.md' } }); fireEvent.click(createButton); @@ -136,7 +144,7 @@ describe('CreateFileModal', () => { render(); const fileNameInput = screen.getByTestId('file-name-input'); - const createButton = screen.getByTestId('confirm-create-button'); + const createButton = screen.getByTestId('confirm-create-file-button'); fireEvent.change(fileNameInput, { target: { value: ' spaced-file.md ' }, @@ -161,7 +169,7 @@ describe('CreateFileModal', () => { render(); const fileNameInput = screen.getByTestId('file-name-input'); - const createButton = screen.getByTestId('confirm-create-button'); + const createButton = screen.getByTestId('confirm-create-file-button'); fireEvent.change(fileNameInput, { target: { value: ' ' } }); @@ -170,7 +178,7 @@ describe('CreateFileModal', () => { }); }); - describe('File name variations', () => { + describe('File Name Variations', () => { it.each([ ['file-with_special.chars (1).md', 'special characters'], ['README', 'no extension'], @@ -180,7 +188,7 @@ describe('CreateFileModal', () => { render(); const fileNameInput = screen.getByTestId('file-name-input'); - const createButton = screen.getByTestId('confirm-create-button'); + const createButton = screen.getByTestId('confirm-create-file-button'); fireEvent.change(fileNameInput, { target: { value: fileName } }); fireEvent.click(createButton); diff --git a/app/src/components/modals/file/CreateFileModal.tsx b/app/src/components/modals/file/CreateFileModal.tsx index 499af0f..f3c7d7e 100644 --- a/app/src/components/modals/file/CreateFileModal.tsx +++ b/app/src/components/modals/file/CreateFileModal.tsx @@ -49,13 +49,13 @@ const CreateFileModal: React.FC = ({ onCreateFile }) => { diff --git a/app/src/components/modals/git/CommitMessageModal.test.tsx b/app/src/components/modals/git/CommitMessageModal.test.tsx index 7138702..099ce93 100644 --- a/app/src/components/modals/git/CommitMessageModal.test.tsx +++ b/app/src/components/modals/git/CommitMessageModal.test.tsx @@ -55,20 +55,26 @@ describe('CommitMessageModal', () => { mockModalContext.setCommitMessageModalVisible.mockClear(); }); - describe('Modal Rendering and Controls', () => { - it('renders modal with form elements when open', () => { + describe('Modal Visibility and Content', () => { + it('renders modal with correct content when opened', () => { render(); expect(screen.getByText('Enter Commit Message')).toBeInTheDocument(); expect(screen.getByTestId('commit-message-input')).toBeInTheDocument(); - expect(screen.getByTestId('cancel-commit-button')).toBeInTheDocument(); - expect(screen.getByTestId('commit-button')).toBeInTheDocument(); + expect( + screen.getByTestId('cancel-commit-message-button') + ).toBeInTheDocument(); + expect( + screen.getByTestId('confirm-commit-message-button') + ).toBeInTheDocument(); }); + }); - it('closes modal when cancel button is clicked', () => { + describe('User Actions', () => { + it('calls onClose when cancel button is clicked', () => { render(); - const cancelButton = screen.getByTestId('cancel-commit-button'); + const cancelButton = screen.getByTestId('cancel-commit-message-button'); fireEvent.click(cancelButton); expect( @@ -77,7 +83,7 @@ describe('CommitMessageModal', () => { }); }); - describe('Form Input and Validation', () => { + describe('Form Validation', () => { it('updates input value when user types', () => { render(); @@ -90,7 +96,7 @@ describe('CommitMessageModal', () => { it('disables commit button when input is empty', () => { render(); - const commitButton = screen.getByTestId('commit-button'); + const commitButton = screen.getByTestId('confirm-commit-message-button'); expect(commitButton).toBeDisabled(); }); @@ -98,18 +104,20 @@ describe('CommitMessageModal', () => { render(); const messageInput = screen.getByTestId('commit-message-input'); - const commitButton = screen.getByTestId('commit-button'); + const commitButton = screen.getByTestId('confirm-commit-message-button'); fireEvent.change(messageInput, { target: { value: 'Test commit' } }); expect(commitButton).not.toBeDisabled(); }); + }); - it('trims whitespace from commit messages', async () => { + describe('Commit and Push Flow', () => { + it('calls onCommitAndPush with trimmed message', async () => { render(); const messageInput = screen.getByTestId('commit-message-input'); - const commitButton = screen.getByTestId('commit-button'); + const commitButton = screen.getByTestId('confirm-commit-message-button'); fireEvent.change(messageInput, { target: { value: ' Update README ' }, @@ -120,14 +128,12 @@ describe('CommitMessageModal', () => { expect(mockOnCommitAndPush).toHaveBeenCalledWith('Update README'); }); }); - }); - describe('Form Submission', () => { - it('calls onCommitAndPush with message when commit button clicked', async () => { + it('calls onCommitAndPush when commit button clicked', async () => { render(); const messageInput = screen.getByTestId('commit-message-input'); - const commitButton = screen.getByTestId('commit-button'); + const commitButton = screen.getByTestId('confirm-commit-message-button'); fireEvent.change(messageInput, { target: { value: 'Fix bug in editor' }, @@ -165,7 +171,7 @@ describe('CommitMessageModal', () => { render(); const messageInput = screen.getByTestId('commit-message-input'); - const commitButton = screen.getByTestId('commit-button'); + const commitButton = screen.getByTestId('confirm-commit-message-button'); fireEvent.change(messageInput, { target: { value: 'Initial commit' } }); fireEvent.click(commitButton); @@ -197,8 +203,8 @@ describe('CommitMessageModal', () => { it('has accessible buttons with proper roles', () => { render(); - const cancelButton = screen.getByTestId('cancel-commit-button'); - const commitButton = screen.getByTestId('commit-button'); + const cancelButton = screen.getByTestId('cancel-commit-message-button'); + const commitButton = screen.getByTestId('confirm-commit-message-button'); // Mantine buttons are semantic HTML buttons expect(cancelButton.tagName).toBe('BUTTON'); diff --git a/app/src/components/modals/git/CommitMessageModal.tsx b/app/src/components/modals/git/CommitMessageModal.tsx index 7eab8cc..df0413c 100644 --- a/app/src/components/modals/git/CommitMessageModal.tsx +++ b/app/src/components/modals/git/CommitMessageModal.tsx @@ -53,13 +53,13 @@ const CommitMessageModal: React.FC = ({