Merge pull request #33162 from overleaf/cd-upload-generated-files-to-filetree

Cd upload generated files to filetree

GitOrigin-RevId: 2d2774e57c42452fba3a2582fde7153ffcde59bf

services/web/test/frontend/features/ide-react/unit/editor/pyodide-worker-client.spec.ts

@@ -1,12 +1,17 @@
 import { expect } from 'chai'
+import sinon from 'sinon'
 import {
   PyodideWorkerClient,
   type LifecycleCallback,
+  type OutputCallback,
 } from '@/features/ide-react/components/editor/python/pyodide-worker-client'
+import type { FileUploader } from '@/features/ide-react/components/editor/python/python-runner'
 import { WorkerMock, createWorker } from './worker-mock'
 
 const BASE_ASSET_PATH = 'https://assets.example.test/'
 
+const fileUploaderStub: FileUploader = () => Promise.resolve([])
+
 describe('PyodideWorkerClient', function () {
   beforeEach(function () {
     WorkerMock.instances.length = 0
@@ -16,6 +21,7 @@ describe('PyodideWorkerClient', function () {
     const client = new PyodideWorkerClient({
       baseAssetPath: BASE_ASSET_PATH,
       createWorker,
+      fileUploader: fileUploaderStub,
     })
     const worker = WorkerMock.instances[0]
 
@@ -50,6 +56,7 @@ describe('PyodideWorkerClient', function () {
     const client = new PyodideWorkerClient({
       baseAssetPath: BASE_ASSET_PATH,
       createWorker,
+      fileUploader: fileUploaderStub,
     })
     const worker = WorkerMock.instances[0]
     worker.emitMessage({ type: 'listening' })
@@ -78,12 +85,45 @@ describe('PyodideWorkerClient', function () {
       onLifecycle: event => {
         lifecycleEvents.push(event)
       },
+      fileUploader: fileUploaderStub,
     })
     const worker = WorkerMock.instances[0]
     worker.emitMessage({ type: 'listening' })
     return { client, worker, lifecycleEvents }
   }
 
+  function setupClientWithUploadTracking(options: {
+    fileUploader: FileUploader
+  }) {
+    const lifecycleEvents: Parameters<LifecycleCallback>[0][] = []
+    const outputCalls: Parameters<OutputCallback>[] = []
+
+    const client = new PyodideWorkerClient({
+      baseAssetPath: BASE_ASSET_PATH,
+      createWorker,
+      onLifecycle: event => {
+        lifecycleEvents.push(event)
+      },
+      onOutput: (...args) => {
+        outputCalls.push(args)
+      },
+      fileUploader: options.fileUploader,
+    })
+    const worker = WorkerMock.instances[0]
+    worker.emitMessage({ type: 'listening' })
+    return { client, worker, lifecycleEvents, outputCalls }
+  }
+
+  async function waitFor(predicate: () => boolean, timeoutMs = 200) {
+    const deadline = Date.now() + timeoutMs
+    while (!predicate()) {
+      if (Date.now() > deadline) {
+        throw new Error('waitFor timed out')
+      }
+      await new Promise(resolve => setTimeout(resolve, 0))
+    }
+  }
+
   it('emits run-finished lifecycle event from run-code-result', function () {
     const { client, worker, lifecycleEvents } =
       setupClientWithLifecycleTracking()
@@ -109,7 +149,7 @@ describe('PyodideWorkerClient', function () {
         executionId: 'exec-3',
         success: true,
         outputs: ['/project/output.txt'],
-        outputFiles: [],
+        failedUploads: [],
       },
     ])
   })
@@ -139,7 +179,7 @@ describe('PyodideWorkerClient', function () {
         executionId: 'exec-4',
         success: true,
         outputs: ['/project/fig1.png', '/project/results/data.csv'],
-        outputFiles: [],
+        failedUploads: [],
       },
     ])
   })
@@ -169,12 +209,12 @@ describe('PyodideWorkerClient', function () {
         executionId: 'exec-5',
         success: true,
         outputs: [],
-        outputFiles: [],
+        failedUploads: [],
       },
     ])
   })
 
-  it('surfaces success and outputFiles from run-code-result', function () {
+  it('surfaces success and outputFiles from run-code-result', async function () {
     const { client, worker, lifecycleEvents } =
       setupClientWithLifecycleTracking()
 
@@ -197,6 +237,10 @@ describe('PyodideWorkerClient', function () {
       ],
     })
 
+    await waitFor(() =>
+      Boolean(lifecycleEvents.find(e => e.type === 'run-finished'))
+    )
+
     const finished = lifecycleEvents.find(e => e.type === 'run-finished')
     expect(finished).to.deep.equal({
       type: 'run-finished',
@@ -204,10 +248,7 @@ describe('PyodideWorkerClient', function () {
       executionId: 'exec-success',
       success: true,
       outputs: ['/project/data.csv', '/project/plot.png'],
-      outputFiles: [
-        { relativePath: 'data.csv', content: csvContent },
-        { relativePath: 'plot.png', content: pngContent },
-      ],
+      failedUploads: [],
     })
   })
 
@@ -236,7 +277,7 @@ describe('PyodideWorkerClient', function () {
       executionId: 'exec-error',
       success: false,
       outputs: [],
-      outputFiles: [],
+      failedUploads: [],
     })
   })
 
@@ -265,7 +306,7 @@ describe('PyodideWorkerClient', function () {
       executionId: 'exec-nowrites',
       success: true,
       outputs: [],
-      outputFiles: [],
+      failedUploads: [],
     })
   })
 
@@ -278,6 +319,7 @@ describe('PyodideWorkerClient', function () {
       onLifecycle: event => {
         lifecycleEvents.push(event)
       },
+      fileUploader: fileUploaderStub,
     })
     const worker = WorkerMock.instances[0]
 
@@ -302,6 +344,7 @@ describe('PyodideWorkerClient', function () {
     const client = new PyodideWorkerClient({
       baseAssetPath: BASE_ASSET_PATH,
       createWorker,
+      fileUploader: fileUploaderStub,
     })
     const worker = WorkerMock.instances[0]
 
@@ -314,11 +357,185 @@ describe('PyodideWorkerClient', function () {
     expect(worker.terminated).to.equal(true)
   })
 
+  context('upload behavior', function () {
+    const successResult = (name: string, relativePath: string) => ({
+      status: 'success' as const,
+      name,
+      relativePath,
+      data: { success: true },
+    })
+    const errorResult = (
+      name: string,
+      relativePath: string,
+      error: string
+    ) => ({
+      status: 'error' as const,
+      name,
+      relativePath,
+      error,
+    })
+
+    function emitRunResult(
+      worker: WorkerMock,
+      executionId: string,
+      outputFiles: Array<{ relativePath: string; content: Uint8Array }>,
+      success = true
+    ) {
+      worker.emitMessage({
+        type: 'run-code-result',
+        fileId: 'main.py',
+        executionId,
+        success,
+        outputs: [],
+        outputFiles,
+      })
+    }
+
+    const findFinished = (
+      lifecycleEvents: Parameters<LifecycleCallback>[0][]
+    ) => lifecycleEvents.find(e => e.type === 'run-finished')
+
+    it('invokes fileUploader with mapped items when run-code-result has output files', async function () {
+      const uploader = sinon
+        .stub()
+        .resolves([successResult('data.csv', 'output/data.csv')])
+      const { worker, lifecycleEvents } = setupClientWithUploadTracking({
+        fileUploader: uploader,
+      })
+
+      emitRunResult(worker, 'exec-up-1', [
+        {
+          relativePath: 'output/data.csv',
+          content: new TextEncoder().encode('a,b\n1,2'),
+        },
+      ])
+      await waitFor(() => Boolean(findFinished(lifecycleEvents)))
+
+      expect(uploader.calledOnce).to.be.true
+      const [items] = uploader.firstCall.args
+      expect(items).to.have.lengthOf(1)
+      expect(items[0].name).to.equal('data.csv')
+      expect(items[0].relativePath).to.equal('output/data.csv')
+      expect(items[0].file).to.be.instanceOf(Blob)
+    })
+
+    it('emits success: true and empty failedUploads when all uploads succeed', async function () {
+      const uploader = sinon
+        .stub()
+        .resolves([
+          successResult('a.csv', 'a.csv'),
+          successResult('b.csv', 'b.csv'),
+        ])
+      const { worker, lifecycleEvents } = setupClientWithUploadTracking({
+        fileUploader: uploader,
+      })
+
+      emitRunResult(worker, 'exec-up-2', [
+        { relativePath: 'a.csv', content: new TextEncoder().encode('1') },
+        { relativePath: 'b.csv', content: new TextEncoder().encode('2') },
+      ])
+      await waitFor(() => Boolean(findFinished(lifecycleEvents)))
+
+      const finished = findFinished(lifecycleEvents)
+      expect(finished).to.deep.include({ success: true, failedUploads: [] })
+    })
+
+    it('flips success to false and lists failed paths when an upload fails', async function () {
+      const uploader = sinon
+        .stub()
+        .resolves([
+          successResult('good.csv', 'good.csv'),
+          errorResult('bad.csv', 'output/bad.csv', 'duplicate_file_name'),
+        ])
+      const { worker, lifecycleEvents, outputCalls } =
+        setupClientWithUploadTracking({ fileUploader: uploader })
+
+      emitRunResult(worker, 'exec-up-3', [
+        { relativePath: 'good.csv', content: new TextEncoder().encode('1') },
+        {
+          relativePath: 'output/bad.csv',
+          content: new TextEncoder().encode('2'),
+        },
+      ])
+      await waitFor(() => Boolean(findFinished(lifecycleEvents)))
+
+      const finished = findFinished(lifecycleEvents)
+      expect(finished).to.deep.include({
+        success: false,
+        failedUploads: ['output/bad.csv'],
+      })
+
+      // user-facing stderr line surfaced via outputCallback
+      expect(outputCalls).to.have.lengthOf(1)
+      const [stream, line] = outputCalls[0]
+      expect(stream).to.equal('stderr')
+      expect(line).to.include('output/bad.csv')
+      expect(line).to.include('duplicate_file_name')
+    })
+
+    it('lists every file in failedUploads and surfaces a single stderr line when fileUploader rejects', async function () {
+      const uploader = sinon.stub().rejects(new Error('network down'))
+      const { worker, lifecycleEvents, outputCalls } =
+        setupClientWithUploadTracking({ fileUploader: uploader })
+
+      emitRunResult(worker, 'exec-up-4', [
+        { relativePath: 'a.csv', content: new TextEncoder().encode('1') },
+        { relativePath: 'b.csv', content: new TextEncoder().encode('2') },
+      ])
+      await waitFor(() => Boolean(findFinished(lifecycleEvents)))
+
+      const finished = findFinished(lifecycleEvents)
+      expect(finished).to.deep.include({ success: false })
+      expect(finished)
+        .to.have.property('failedUploads')
+        .that.has.members(['a.csv', 'b.csv'])
+
+      expect(outputCalls).to.have.lengthOf(1)
+      const [stream, line] = outputCalls[0]
+      expect(stream).to.equal('stderr')
+      expect(line).to.include('network down')
+    })
+
+    it('does not invoke fileUploader when run-code-result has success: false', async function () {
+      const uploader = sinon.stub().resolves([])
+      const { worker, lifecycleEvents } = setupClientWithUploadTracking({
+        fileUploader: uploader,
+      })
+
+      emitRunResult(
+        worker,
+        'exec-up-5',
+        [{ relativePath: 'a.csv', content: new TextEncoder().encode('1') }],
+        false
+      )
+      await waitFor(() => Boolean(findFinished(lifecycleEvents)))
+
+      expect(uploader.called).to.be.false
+      expect(findFinished(lifecycleEvents)).to.deep.include({
+        success: false,
+        failedUploads: [],
+      })
+    })
+
+    it('does not invoke fileUploader when outputFiles is empty', async function () {
+      const uploader = sinon.stub().resolves([])
+      const { worker, lifecycleEvents } = setupClientWithUploadTracking({
+        fileUploader: uploader,
+      })
+
+      emitRunResult(worker, 'exec-up-6', [])
+      await waitFor(() => Boolean(findFinished(lifecycleEvents)))
+
+      expect(uploader.called).to.be.false
+    })
+  })
+
   describe('reset', function () {
     it('terminates the current worker and creates a new one', function () {
       const client = new PyodideWorkerClient({
         baseAssetPath: BASE_ASSET_PATH,
         createWorker,
+        fileUploader: fileUploaderStub,
       })
       const originalWorker = WorkerMock.instances[0]
       originalWorker.emitMessage({ type: 'listening' })
@@ -334,6 +551,7 @@ describe('PyodideWorkerClient', function () {
       const client = new PyodideWorkerClient({
         baseAssetPath: BASE_ASSET_PATH,
         createWorker,
+        fileUploader: fileUploaderStub,
       })
       const originalWorker = WorkerMock.instances[0]
       originalWorker.emitMessage({ type: 'listening' })
@@ -357,6 +575,7 @@ describe('PyodideWorkerClient', function () {
       const client = new PyodideWorkerClient({
         baseAssetPath: BASE_ASSET_PATH,
         createWorker,
+        fileUploader: fileUploaderStub,
       })
       const originalWorker = WorkerMock.instances[0]
       originalWorker.emitMessage({ type: 'listening' })
@@ -389,6 +608,7 @@ describe('PyodideWorkerClient', function () {
       const client = new PyodideWorkerClient({
         baseAssetPath: BASE_ASSET_PATH,
         createWorker,
+        fileUploader: fileUploaderStub,
       })
       const originalWorker = WorkerMock.instances[0]
       originalWorker.emitMessage({ type: 'listening' })

services/web/test/frontend/features/ide-react/unit/editor/python-runner.spec.ts

@@ -2,8 +2,10 @@ import { expect } from 'chai'
 import sinon from 'sinon'
 import {
   PythonRunner,
+  PythonRunnerState,
   DEFAULT_STATE,
   ExecutionContext,
+  type FileUploader,
 } from '@/features/ide-react/components/editor/python/python-runner'
 import { WorkerMock, createWorker } from './worker-mock'
 
@@ -14,6 +16,7 @@ function createRunner(
   overrides: {
     fileId?: string
     getExecutionContext?: () => Promise<ExecutionContext | null>
+    fileUploader?: FileUploader
   } = {}
 ) {
   const fileId = overrides.fileId ?? FILE_ID
@@ -25,11 +28,14 @@ function createRunner(
         files: [{ relativePath: 'main.py', content: 'print("hello")' }],
       }))
 
+  const fileUploader = overrides.fileUploader ?? sinon.stub().resolves([])
+
   const runner = new PythonRunner(
     fileId,
     BASE_ASSET_PATH,
     getExecutionContext,
-    createWorker
+    createWorker,
+    fileUploader
   )
   return runner
 }
@@ -42,6 +48,24 @@ function initAndLoad(runner: PythonRunner) {
   return worker
 }
 
+function waitForState(
+  runner: PythonRunner,
+  predicate: (state: PythonRunnerState) => boolean
+): Promise<PythonRunnerState> {
+  return new Promise(resolve => {
+    if (predicate(runner.getState())) {
+      resolve(runner.getState())
+      return
+    }
+    const unsubscribe = runner.subscribe(() => {
+      if (predicate(runner.getState())) {
+        unsubscribe()
+        resolve(runner.getState())
+      }
+    })
+  })
+}
+
 describe('PythonRunner', function () {
   beforeEach(function () {
     WorkerMock.instances.length = 0
@@ -110,9 +134,12 @@ describe('PythonRunner', function () {
         type: 'run-code-result',
         fileId: FILE_ID,
         executionId: runMsg.executionId,
+        success: true,
         outputs: [],
+        outputFiles: [],
       })
 
+      await waitForState(runner, s => s.status === 'finished')
       expect(runner.getState().status).to.equal('finished')
     })
 
@@ -133,7 +160,9 @@ describe('PythonRunner', function () {
         type: 'run-code-result',
         fileId: FILE_ID,
         executionId: runMsg.executionId,
+        success: true,
         outputs: [],
+        outputFiles: [],
       })
       expect(runner.getState().output).to.deep.equal([
         { stream: 'stdout', line: 'first run output' },