Add tests for image uploading

Summary:
Actually add tests for all the image uploading features added in this stack.

We need to hardcode image uploading to be enabled for our tests, since it may be disabled in production / OSS builds.

These tests go over pasting/dragging/choosing files, making sure it sends the data to the server, and the placeholder&replacement system for the final uploaded links

Reviewed By: quark-zju

Differential Revision: D43408510

fbshipit-source-id: d581bb1df794e09bef15d944b52a01659f74bfb2

addons/isl/src/ImageUpload.tsx

@@ -134,12 +134,14 @@ export function FilePicker({uploadFiles}: {uploadFiles: (files: Array<File>) =>
         type="file"
         accept="image/*,video/*"
         className="choose-file"
+        data-testid="attach-file-input"
         id={id}
         multiple
         onChange={event => {
           if (event.target.files) {
             uploadFiles([...event.target.files]);
           }
+          event.target.files = null;
         }}
       />
       <label htmlFor={id}>
@@ -147,7 +149,7 @@ export function FilePicker({uploadFiles}: {uploadFiles: (files: Array<File>) =>
           title={t(
             'Choose image or video files to upload. Drag & Drop and Pasting images or videos is also supported.',
           )}>
-          <VSCodeButton appearance="icon">
+          <VSCodeButton appearance="icon" data-testid="attach-file-button">
             <PaperclipIcon />
           </VSCodeButton>
         </Tooltip>

addons/isl/src/TextArea.tsx

@@ -111,7 +111,11 @@ export function CommitInfoField({
   // The gh cli does not support uploading images for commit messages,
   // see https://github.com/cli/cli/issues/1895#issuecomment-718899617
   // for now, this is internal-only.
-  const supportsImageUpload = which === 'description' && Internal.supportsImageUpload === true;
+  const supportsImageUpload =
+    which === 'description' &&
+    (Internal.supportsImageUpload === true ||
+      // image upload is always enabled in tests
+      process.env.NODE_ENV === 'test');
 
   const uploadFiles = useUploadFilesCallback(ref);
 

addons/isl/src/__mocks__/MessageBus.ts

@@ -11,7 +11,7 @@ import type {Disposable} from '../types';
 /** This fake implementation of MessageBus expects you to manually simulate messages from the server */
 export class TestingEventBus implements MessageBus {
   public handlers: Array<(e: MessageEvent<string>) => void> = [];
-  public sent: Array<string> = [];
+  public sent: Array<string | ArrayBuffer> = [];
   onMessage(handler: (event: MessageEvent<string>) => void | Promise<void>): Disposable {
     this.handlers.push(handler);
     // eslint-disable-next-line @typescript-eslint/no-empty-function

addons/isl/src/__tests__/ClientToServer.test.ts

@@ -0,0 +1,72 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+import clientToServerAPI from '../ClientToServerAPI';
+import {
+  getLastMessagesSentToServer,
+  resetTestMessages,
+  simulateMessageFromServer,
+} from '../testUtils';
+import {nextTick} from 'shared/testUtils';
+
+jest.mock('../MessageBus');
+
+describe('ClientToServer', () => {
+  beforeEach(() => {
+    resetTestMessages();
+  });
+
+  describe('nextMessageMatching', () => {
+    it('resolves when it sees a matching message', async () => {
+      let isResolved = false;
+      const matchingPromise = clientToServerAPI.nextMessageMatching(
+        'uploadFileResult',
+        message => message.id === '1234',
+      );
+
+      matchingPromise.then(() => {
+        isResolved = true;
+      });
+
+      simulateMessageFromServer({type: 'beganLoadingMoreCommits'}); // doesn't match type
+      simulateMessageFromServer({type: 'uploadFileResult', result: {value: 'hi'}, id: '9999'}); // doesn't match predicate
+      await nextTick();
+      expect(isResolved).toEqual(false);
+
+      simulateMessageFromServer({type: 'uploadFileResult', result: {value: 'hi'}, id: '1234'}); // matches
+      expect(matchingPromise).resolves.toEqual({
+        type: 'uploadFileResult',
+        result: {value: 'hi'},
+        id: '1234',
+      });
+
+      simulateMessageFromServer({type: 'uploadFileResult', result: {value: 'hi'}, id: '1234'}); // doesn't crash or anything if another message would match
+    });
+  });
+
+  describe('postMessageWithPayload', () => {
+    it('sends two messages, one with payload', () => {
+      clientToServerAPI.postMessageWithPayload(
+        {type: 'uploadFile', filename: 'test.png', id: '0'},
+        new Uint8Array([1, 2, 3, 4]).buffer,
+      );
+
+      const [json, binary] = getLastMessagesSentToServer(2);
+      expect(json).toEqual(
+        JSON.stringify({
+          __rpcType: 'object',
+          type: 'uploadFile',
+          filename: 'test.png',
+          id: '0',
+          hasBinaryPayload: true,
+        }),
+      );
+      expect(binary).toBeInstanceOf(ArrayBuffer);
+      expect([...new Uint8Array(binary as ArrayBuffer)]).toEqual([1, 2, 3, 4]);
+    });
+  });
+});

addons/isl/src/__tests__/ImageUpload.test.tsx

@@ -0,0 +1,320 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+import App from '../App';
+import {
+  COMMIT,
+  CommitInfoTestUtils,
+  expectMessageNOTSentToServer,
+  expectMessageSentToServer,
+  fireMouseEvent,
+  getLastBinaryMessageSentToServer,
+  resetTestMessages,
+  simulateCommits,
+  simulateMessageFromServer,
+} from '../testUtils';
+import {fireEvent, render, waitFor, screen} from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import {act} from 'react-dom/test-utils';
+import {nextTick} from 'shared/testUtils';
+import * as utils from 'shared/utils';
+
+jest.mock('../MessageBus');
+
+describe('Image upload inside TextArea ', () => {
+  beforeEach(() => {
+    resetTestMessages();
+  });
+
+  beforeEach(() => {
+    render(<App />);
+    act(() => {
+      expectMessageSentToServer({
+        type: 'subscribeSmartlogCommits',
+        subscriptionID: expect.anything(),
+      });
+      simulateCommits({
+        value: [
+          COMMIT('1', 'some public base', '0', {phase: 'public'}),
+          COMMIT('a', 'My Commit', '1', {isHead: true}),
+        ],
+      });
+    });
+    act(() => {
+      CommitInfoTestUtils.clickCommitMode();
+    });
+  });
+
+  const mockFile = {
+    name: 'file.png',
+    arrayBuffer: () => Promise.resolve(new Uint8Array([0, 1, 2]).buffer),
+  } as File;
+
+  const dataTransfer = {
+    files: [mockFile] as unknown as FileList,
+  } as DataTransfer;
+
+  describe('Drag and drop image', () => {
+    it('renders highlight while dragging image', () => {
+      const textarea = CommitInfoTestUtils.getDescriptionEditor();
+
+      act(() => void fireMouseEvent('dragenter', textarea, 0, 0, {dataTransfer}));
+      expect(document.querySelector('.hovering-to-drop')).not.toBeNull();
+      act(() => void fireMouseEvent('dragleave', textarea, 0, 0, {dataTransfer}));
+      expect(document.querySelector('.hovering-to-drop')).toBeNull();
+    });
+
+    it('does not try to upload other things being dragged', () => {
+      const textarea = CommitInfoTestUtils.getDescriptionEditor();
+      act(() => {
+        fireMouseEvent('dragenter', textarea, 0, 0, {
+          dataTransfer: {
+            files: [],
+            items: [],
+          } as unknown as DataTransfer,
+        });
+      }); // drag without files is ignored
+      expect(document.querySelector('.hovering-to-drop')).toBeNull();
+    });
+
+    it('lets you drag an image to upload it', async () => {
+      const textarea = CommitInfoTestUtils.getDescriptionEditor();
+      act(() => void fireMouseEvent('dragenter', textarea, 0, 0, {dataTransfer}));
+      act(() => {
+        fireMouseEvent('drop', textarea, 0, 0, {dataTransfer});
+      });
+
+      await waitFor(() => {
+        expectMessageSentToServer(expect.objectContaining({type: 'uploadFile'}));
+      });
+    });
+  });
+
+  describe('Paste image to upload', () => {
+    it('lets you paste an image to upload it', async () => {
+      const textarea = CommitInfoTestUtils.getDescriptionEditor();
+      act(() => {
+        fireEvent.paste(textarea, {clipboardData: dataTransfer});
+      });
+      await waitFor(() => {
+        expectMessageSentToServer(expect.objectContaining({type: 'uploadFile'}));
+      });
+    });
+    it('pastes without images are handled normally', async () => {
+      const textarea = CommitInfoTestUtils.getDescriptionEditor();
+      act(() => void fireEvent.paste(textarea));
+      await nextTick(); // allow file upload to await arrayBuffer()
+      expectMessageNOTSentToServer(expect.objectContaining({type: 'uploadFile'}));
+    });
+  });
+
+  describe('file picker to upload file', () => {
+    it('lets you pick a file to upload', async () => {
+      const uploadButton = screen.getByTestId('attach-file-input');
+      act(() => {
+        userEvent.upload(uploadButton, [mockFile]);
+      });
+
+      await waitFor(() => {
+        expectMessageSentToServer(expect.objectContaining({type: 'uploadFile'}));
+      });
+    });
+  });
+
+  describe('Image upload UI', () => {
+    let randomIdSpy: jest.SpyInstance;
+    beforeEach(() => {
+      randomIdSpy = jest.spyOn(utils, 'randomId');
+    });
+    afterEach(() => {
+      randomIdSpy.mockClear();
+    });
+
+    async function startFileUpload(id: string) {
+      randomIdSpy.mockImplementationOnce(() => id);
+      const uploadButton = screen.getByTestId('attach-file-input');
+      act(() => void userEvent.upload(uploadButton, [mockFile]));
+      await waitFor(() =>
+        expectMessageSentToServer(expect.objectContaining({type: 'uploadFile', id})),
+      );
+    }
+
+    async function simulateUploadSucceeded(id: string) {
+      await act(async () => {
+        simulateMessageFromServer({
+          type: 'uploadFileResult',
+          id,
+          result: {value: `https://image.example.com/${id}`},
+        });
+        await nextTick();
+      });
+    }
+
+    async function simulateUploadFailed(id: string) {
+      await act(async () => {
+        simulateMessageFromServer({
+          type: 'uploadFileResult',
+          id,
+          result: {error: new Error('upload failed')},
+        });
+        await nextTick();
+      });
+    }
+
+    const {descriptionTextContent, getDescriptionEditor} = CommitInfoTestUtils;
+
+    it('shows placeholder when uploading an image', async () => {
+      expect(descriptionTextContent()).not.toContain('Uploading');
+      await startFileUpload('1111');
+      expect(descriptionTextContent()).toContain('Uploading #1');
+    });
+
+    it('sends a message to the server to upload the file', async () => {
+      jest.spyOn(utils, 'randomId').mockImplementation(() => '1111');
+      await startFileUpload('1111');
+      expectMessageSentToServer({
+        type: 'uploadFile',
+        filename: 'file.png',
+        hasBinaryPayload: true,
+        id: '1111',
+      });
+      const binary = getLastBinaryMessageSentToServer();
+      expect(binary).toEqual(new Uint8Array([1, 2, 3, 4]).buffer);
+    });
+
+    it('removes placeholder when upload succeeds', async () => {
+      await startFileUpload('1111');
+      expect(descriptionTextContent()).toContain('Uploading #1');
+      await simulateUploadSucceeded('1111');
+      expect(descriptionTextContent()).not.toContain('Uploading #1');
+      expect(descriptionTextContent()).toContain('https://image.example.com/1111');
+    });
+
+    it('removes placeholder when upload fails', async () => {
+      await startFileUpload('1111');
+      expect(descriptionTextContent()).toContain('Uploading #1');
+      await simulateUploadFailed('1111');
+      expect(descriptionTextContent()).not.toContain('Uploading #1');
+      expect(descriptionTextContent()).not.toContain('https://image.example.com');
+    });
+
+    it('shows progress of ongoing uploads', async () => {
+      await startFileUpload('1111');
+      expect(screen.getByText('Uploading 1 file')).toBeInTheDocument();
+    });
+
+    it('click to cancel upload', async () => {
+      await startFileUpload('1111');
+      expect(screen.getByText('Uploading 1 file')).toBeInTheDocument();
+      act(() => {
+        fireEvent.mouseOver(screen.getByText('Uploading 1 file'));
+      });
+      expect(screen.getByText('Click to cancel')).toBeInTheDocument();
+      act(() => {
+        fireEvent.click(screen.getByText('Click to cancel'));
+      });
+
+      expect(descriptionTextContent()).not.toContain('Uploading #1');
+      expect(screen.queryByText('Uploading 1 file')).not.toBeInTheDocument();
+    });
+
+    it('clears hover state when cancelling', async () => {
+      await startFileUpload('1111');
+      act(() => void fireEvent.mouseOver(screen.getByText('Uploading 1 file')));
+      act(() => void fireEvent.click(screen.getByText('Click to cancel')));
+      await startFileUpload('2222');
+      expect(screen.queryByText('Uploading 1 file')).toBeInTheDocument();
+    });
+
+    it('handles multiple placeholders', async () => {
+      await startFileUpload('1111');
+      expect(screen.getByText('Uploading 1 file')).toBeInTheDocument();
+      await startFileUpload('2222');
+      expect(screen.getByText('Uploading 2 files')).toBeInTheDocument();
+
+      expect(descriptionTextContent()).toContain('Uploading #1');
+      expect(descriptionTextContent()).toContain('Uploading #2');
+      await simulateUploadSucceeded('1111');
+      expect(descriptionTextContent()).not.toContain('Uploading #1');
+      expect(descriptionTextContent()).toContain('Uploading #2');
+
+      expect(descriptionTextContent()).toContain('https://image.example.com/1111');
+      expect(descriptionTextContent()).not.toContain('https://image.example.com/2222');
+
+      await simulateUploadSucceeded('2222');
+      expect(descriptionTextContent()).not.toContain('Uploading #2');
+      expect(descriptionTextContent()).toContain('https://image.example.com/2222');
+    });
+
+    it('inserts whitespace before inserted placeholder when necessary', async () => {
+      act(() => {
+        userEvent.type(getDescriptionEditor(), 'Hello!\n');
+        //                                     ^ cursor
+        getDescriptionEditor().selectionStart = 6;
+        getDescriptionEditor().selectionEnd = 6;
+      });
+      await startFileUpload('1111');
+      expect(descriptionTextContent()).toEqual('Hello! 【 Uploading #1 】\n');
+      //                                       ^ inserted space  ^ no extra space
+    });
+
+    it('inserts whitespace after inserted placeholder when necessary', async () => {
+      act(() => {
+        userEvent.type(getDescriptionEditor(), 'Hello!\n');
+        //                                          ^ cursor
+        getDescriptionEditor().selectionStart = 0;
+        getDescriptionEditor().selectionEnd = 0;
+      });
+      await startFileUpload('1111');
+      expect(descriptionTextContent()).toEqual('【 Uploading #1 】 Hello!\n');
+      //                                        ^ no space       ^ inserted space
+    });
+
+    it('preserves selection when setting placeholders', async () => {
+      act(() => {
+        userEvent.type(getDescriptionEditor(), 'Hello, world!\n');
+        //                                            ^-----^ selection
+        getDescriptionEditor().selectionStart = 2;
+        getDescriptionEditor().selectionEnd = 8;
+      });
+      await startFileUpload('1111');
+      expect(descriptionTextContent()).toEqual('He 【 Uploading #1 】 orld!\n');
+      //                                          ^ inserted spaces ^
+
+      // now cursor is after Uploading
+      expect(getDescriptionEditor().selectionStart).toEqual(20);
+      expect(getDescriptionEditor().selectionEnd).toEqual(20);
+    });
+
+    it('preserves selection when replacing placeholders', async () => {
+      act(() => {
+        userEvent.type(getDescriptionEditor(), 'fob\nbar\nbaz');
+        //                                               ^ cursor
+        getDescriptionEditor().selectionStart = 4;
+        getDescriptionEditor().selectionEnd = 4;
+      });
+      await startFileUpload('1111');
+      expect(descriptionTextContent()).toEqual('fob\n【 Uploading #1 】 bar\nbaz');
+      //                     start new selection: ^--------------------------^
+      getDescriptionEditor().selectionStart = 2;
+      getDescriptionEditor().selectionEnd = 26;
+      // make sure my indices are correct
+      expect(descriptionTextContent()[getDescriptionEditor().selectionStart]).toEqual('b');
+      expect(descriptionTextContent()[getDescriptionEditor().selectionEnd]).toEqual('a');
+
+      await simulateUploadSucceeded('1111');
+      expect(descriptionTextContent()).toEqual('fob\nhttps://image.example.com/1111 bar\nbaz');
+      //                 selection is preserved:  ^---------------------------------------^
+
+      // now cursor is after Uploading
+      expect(getDescriptionEditor().selectionStart).toEqual(2);
+      expect(getDescriptionEditor().selectionEnd).toEqual(40);
+      expect(descriptionTextContent()[getDescriptionEditor().selectionStart]).toEqual('b');
+      expect(descriptionTextContent()[getDescriptionEditor().selectionEnd]).toEqual('a');
+    });
+  });
+});

addons/isl/src/testUtils.tsx

@@ -8,6 +8,7 @@
 import type {TestingEventBus} from './__mocks__/MessageBus';
 import type {
   ClientToServerMessage,
+  ClientToServerMessageWithPayload,
   CommitInfo,
   Hash,
   Result,
@@ -28,11 +29,36 @@ export function simulateMessageFromServer(message: ServerToClientMessage): void
   testMessageBus.simulateMessage(serializeToString(message));
 }
 
-export function expectMessageSentToServer(message: Partial<ClientToServerMessage>): void {
-  expect(testMessageBus.sent.map(deserializeFromString)).toContainEqual(message);
+export function expectMessageSentToServer(
+  message: Partial<ClientToServerMessage | ClientToServerMessageWithPayload>,
+): void {
+  expect(
+    testMessageBus.sent
+      .filter((msg: unknown): msg is string => !(msg instanceof ArrayBuffer))
+      .map(deserializeFromString),
+  ).toContainEqual(message);
 }
 export function expectMessageNOTSentToServer(message: Partial<ClientToServerMessage>): void {
-  expect(testMessageBus.sent.map(deserializeFromString)).not.toContainEqual(message);
+  expect(
+    testMessageBus.sent
+      .filter((msg: unknown): msg is string => !(msg instanceof ArrayBuffer))
+      .map(deserializeFromString),
+  ).not.toContainEqual(message);
+}
+
+/**
+ * Return last `num` raw messages sent to the server.
+ * Normal messages will be stingified JSON.
+ * Binary messages with be ArrayBuffers.
+ */
+export function getLastMessagesSentToServer(num: number): Array<string | ArrayBuffer> {
+  return testMessageBus.sent.slice(-num);
+}
+
+export function getLastBinaryMessageSentToServer(): ArrayBuffer | undefined {
+  return testMessageBus.sent.find(
+    (message): message is ArrayBuffer => message instanceof ArrayBuffer,
+  );
 }
 
 export function simulateServerDisconnected(): void {
@@ -144,13 +170,14 @@ export const TEST_COMMIT_HISTORY = [
   COMMIT('1', 'some public base', '0', {phase: 'public'}),
 ];
 
-const fireMouseEvent = function (
+export const fireMouseEvent = function (
   type: string,
   elem: EventTarget,
   centerX: number,
   centerY: number,
+  additionalProperties?: Partial<MouseEvent | InputEvent>,
 ) {
-  const evt = document.createEvent('MouseEvents');
+  const evt = document.createEvent('MouseEvents') as Writable<MouseEvent & InputEvent>;
   evt.initMouseEvent(
     type,
     true,
@@ -168,7 +195,12 @@ const fireMouseEvent = function (
     0,
     elem,
   );
-  (evt as Writable<DragEvent>).dataTransfer = {} as DataTransfer;
+  evt.dataTransfer = {} as DataTransfer;
+  if (additionalProperties != null) {
+    for (const [key, value] of Object.entries(additionalProperties)) {
+      (evt as Record<string, unknown>)[key] = value;
+    }
+  }
   return elem.dispatchEvent(evt);
 };