From 3f50a48d9b92f525e54853d2d7aafc1e64093027 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Mon, 17 Feb 2025 09:32:16 +0100 Subject: [PATCH 1/2] Don't make tests unnecessarily async --- .../src/utils/test/upload-media.ts | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/packages/media-utils/src/utils/test/upload-media.ts b/packages/media-utils/src/utils/test/upload-media.ts index ee6276557ed794..b0a83504c43498 100644 --- a/packages/media-utils/src/utils/test/upload-media.ts +++ b/packages/media-utils/src/utils/test/upload-media.ts @@ -26,10 +26,10 @@ describe( 'uploadMedia', () => { jest.clearAllMocks(); } ); - it( 'should do nothing on no files', async () => { + it( 'should do nothing on no files', () => { const onError = jest.fn(); const onFileChange = jest.fn(); - await uploadMedia( { + uploadMedia( { filesList: [], onError, onFileChange, @@ -39,10 +39,10 @@ describe( 'uploadMedia', () => { expect( uploadToServer ).not.toHaveBeenCalled(); } ); - it( 'should error if allowedTypes contains a partial mime type and the validation fails', async () => { + it( 'should error if allowedTypes contains a partial mime type and the validation fails', () => { const onError = jest.fn(); const onFileChange = jest.fn(); - await uploadMedia( { + uploadMedia( { allowedTypes: [ 'image' ], filesList: [ xmlFile ], onError, @@ -60,10 +60,10 @@ describe( 'uploadMedia', () => { expect( uploadToServer ).not.toHaveBeenCalled(); } ); - it( 'should error if allowedTypes contains a complete mime type and the validation fails', async () => { + it( 'should error if allowedTypes contains a complete mime type and the validation fails', () => { const onError = jest.fn(); const onFileChange = jest.fn(); - await uploadMedia( { + uploadMedia( { allowedTypes: [ 'image/gif' ], filesList: [ imageFile ], onError, @@ -81,10 +81,10 @@ describe( 'uploadMedia', () => { expect( uploadToServer ).not.toHaveBeenCalled(); } ); - it( 'should work if allowedTypes contains a complete mime type and the validation succeeds', async () => { + it( 'should work if allowedTypes contains a complete mime type and the validation succeeds', () => { const onError = jest.fn(); const onFileChange = jest.fn(); - await uploadMedia( { + uploadMedia( { allowedTypes: [ 'image/jpeg' ], filesList: [ imageFile ], onError, @@ -96,10 +96,10 @@ describe( 'uploadMedia', () => { expect( uploadToServer ).toHaveBeenCalled(); } ); - it( 'should error if allowedTypes contains multiple types and the validation fails', async () => { + it( 'should error if allowedTypes contains multiple types and the validation fails', () => { const onError = jest.fn(); const onFileChange = jest.fn(); - await uploadMedia( { + uploadMedia( { allowedTypes: [ 'video', 'image' ], filesList: [ xmlFile ], onError, @@ -117,10 +117,10 @@ describe( 'uploadMedia', () => { expect( uploadToServer ).not.toHaveBeenCalled(); } ); - it( 'should work if allowedTypes contains multiple types and the validation succeeds', async () => { + it( 'should work if allowedTypes contains multiple types and the validation succeeds', () => { const onError = jest.fn(); const onFileChange = jest.fn(); - await uploadMedia( { + uploadMedia( { allowedTypes: [ 'video', 'image' ], filesList: [ imageFile ], onError, @@ -132,10 +132,10 @@ describe( 'uploadMedia', () => { expect( uploadToServer ).toHaveBeenCalled(); } ); - it( 'should only fail the invalid file and still allow others to succeed when uploading multiple files', async () => { + it( 'should only fail the invalid file and still allow others to succeed when uploading multiple files', () => { const onError = jest.fn(); const onFileChange = jest.fn(); - await uploadMedia( { + uploadMedia( { allowedTypes: [ 'image' ], filesList: [ imageFile, xmlFile ], onError, @@ -154,10 +154,10 @@ describe( 'uploadMedia', () => { expect( uploadToServer ).toHaveBeenCalledTimes( 1 ); } ); - it( 'should error if the file size is greater than the maximum', async () => { + it( 'should error if the file size is greater than the maximum', () => { const onError = jest.fn(); const onFileChange = jest.fn(); - await uploadMedia( { + uploadMedia( { allowedTypes: [ 'image' ], filesList: [ imageFile ], maxUploadFileSize: 1, @@ -177,9 +177,9 @@ describe( 'uploadMedia', () => { expect( uploadToServer ).not.toHaveBeenCalled(); } ); - it( 'should call error handler with the correct error object if file type is not allowed for user', async () => { + it( 'should call error handler with the correct error object if file type is not allowed for user', () => { const onError = jest.fn(); - await uploadMedia( { + uploadMedia( { allowedTypes: [ 'image' ], filesList: [ imageFile ], onError, @@ -197,9 +197,9 @@ describe( 'uploadMedia', () => { expect( uploadToServer ).not.toHaveBeenCalled(); } ); - it( 'should throw error when multiple files are selected in single file upload mode', async () => { + it( 'should throw error when multiple files are selected in single file upload mode', () => { const onError = jest.fn(); - await uploadMedia( { + uploadMedia( { filesList: [ imageFile, xmlFile ], onError, multiple: false, From 6e1e1185d77fdcd1c8ee00f84e0c6f3c676aa8a8 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Mon, 17 Feb 2025 09:51:10 +0100 Subject: [PATCH 2/2] Add test for new error behavior --- .../src/utils/test/upload-media.ts | 24 +++++++++++++++++++ .../media-utils/src/utils/upload-media.ts | 1 + 2 files changed, 25 insertions(+) diff --git a/packages/media-utils/src/utils/test/upload-media.ts b/packages/media-utils/src/utils/test/upload-media.ts index b0a83504c43498..66cdeefc53039f 100644 --- a/packages/media-utils/src/utils/test/upload-media.ts +++ b/packages/media-utils/src/utils/test/upload-media.ts @@ -210,4 +210,28 @@ describe( 'uploadMedia', () => { ); expect( uploadToServer ).not.toHaveBeenCalled(); } ); + + it( 'should return error that is not an Error object', () => { + ( uploadToServer as jest.Mock ).mockImplementation( () => { + throw { + code: 'fetch_error', + message: 'You are probably offline.', + }; + } ); + + const onError = jest.fn(); + uploadMedia( { + filesList: [ imageFile ], + onError, + multiple: false, + } ); + + expect( onError ).toHaveBeenCalledWith( + new UploadError( { + code: 'GENERAL', + message: 'You are probably offline.', + file: imageFile, + } ) + ); + } ); } ); diff --git a/packages/media-utils/src/utils/upload-media.ts b/packages/media-utils/src/utils/upload-media.ts index c7ba9d5dbb90b5..3b49e317ffb9e4 100644 --- a/packages/media-utils/src/utils/upload-media.ts +++ b/packages/media-utils/src/utils/upload-media.ts @@ -143,6 +143,7 @@ export function uploadMedia( { // Reset to empty on failure. setAndUpdateFiles( index, null ); + // @wordpress/api-fetch throws any response that isn't in the 200 range as-is. let message: string; if ( typeof error === 'object' &&