From 01bb396637639a8999eb7a0d5d332d61e739447a Mon Sep 17 00:00:00 2001 From: Arjun Attam Date: Mon, 4 Nov 2024 16:08:35 +0530 Subject: [PATCH] fix: move worker video download to reporter (#223) --- .changeset/brown-ligers-deliver.md | 5 + src/fixture/index.ts | 14 +- src/fixture/workerInfo.ts | 34 +++-- src/reporter.ts | 236 ++++++++++++++++++++--------- src/types/index.ts | 13 -- 5 files changed, 198 insertions(+), 104 deletions(-) create mode 100644 .changeset/brown-ligers-deliver.md diff --git a/.changeset/brown-ligers-deliver.md b/.changeset/brown-ligers-deliver.md new file mode 100644 index 0000000..89269d1 --- /dev/null +++ b/.changeset/brown-ligers-deliver.md @@ -0,0 +1,5 @@ +--- +"appwright": patch +--- + +fix: move worker video download to reporter diff --git a/src/fixture/index.ts b/src/fixture/index.ts index f84e750..8dca8c6 100644 --- a/src/fixture/index.ts +++ b/src/fixture/index.ts @@ -7,10 +7,8 @@ import { AppwrightConfig, } from "../types"; import { Device } from "../device"; -import { createDeviceProvider, getProviderClass } from "../providers"; -import { logger } from "../logger"; +import { createDeviceProvider } from "../providers"; import { WorkerInfoStore } from "./workerInfo"; -import { basePath } from "../utils"; type TestLevelFixtures = { /** @@ -66,22 +64,20 @@ export const test = base.extend({ if (!sessionId) { throw new Error("Worker must have a sessionId."); } + const providerName = (project as FullProject).use.device + ?.provider; const afterSession = new Date(); const workerInfoStore = new WorkerInfoStore(); await workerInfoStore.saveWorkerStartTime( workerIndex, sessionId, + providerName!, beforeSession, afterSession, ); await use(device); + await workerInfoStore.saveWorkerEndTime(workerIndex, new Date()); await device.close(); - logger.log(`Teardown for worker ${workerIndex}, will download video`); - const providerName = (project as FullProject).use.device - ?.provider; - const providerClass = getProviderClass(providerName!); - const fileName = `worker-${workerIndex}-video`; - await providerClass.downloadVideo(sessionId, basePath(), fileName); }, { scope: "worker" }, ], diff --git a/src/fixture/workerInfo.ts b/src/fixture/workerInfo.ts index c0daab7..1514205 100644 --- a/src/fixture/workerInfo.ts +++ b/src/fixture/workerInfo.ts @@ -7,9 +7,10 @@ type TestInWorkerInfo = { startTime: string; }; -type WorkerInfo = { +export type WorkerInfo = { idx: number; sessionId?: string | undefined; + providerName?: string | undefined; startTime?: | { // Dates stored as ISO datetime strings @@ -17,6 +18,7 @@ type WorkerInfo = { afterAppiumSession: string; } | undefined; + endTime?: string | undefined; tests: TestInWorkerInfo[]; }; @@ -58,29 +60,43 @@ export class WorkerInfoStore { async saveWorkerStartTime( idx: number, sessionId: string, + providerName: string, beforeAppiumSession: Date, afterAppiumSession: Date, ) { let info = await this.getWorkerFromDisk(idx); + const delta = { + providerName, + sessionId, + startTime: { + beforeAppiumSession: beforeAppiumSession.toISOString(), + afterAppiumSession: afterAppiumSession.toISOString(), + }, + }; if (!info) { info = { + ...delta, idx, - sessionId, - startTime: { - beforeAppiumSession: beforeAppiumSession.toISOString(), - afterAppiumSession: afterAppiumSession.toISOString(), - }, tests: [], }; } else { - info.startTime = { - beforeAppiumSession: beforeAppiumSession.toISOString(), - afterAppiumSession: afterAppiumSession.toISOString(), + info = { + ...info, + ...delta, }; } return this.saveWorkerToDisk(idx, info); } + async saveWorkerEndTime(idx: number, endTime: Date) { + let info = await this.getWorkerFromDisk(idx); + if (!info) { + throw new Error(`Worker info not found for idx: ${idx}`); + } + info.endTime = endTime.toISOString(); + return this.saveWorkerToDisk(idx, info); + } + async saveTestStartTime(idx: number, testTitle: string, startTime: Date) { let info = await this.getWorkerFromDisk(idx); if (!info) { diff --git a/src/reporter.ts b/src/reporter.ts index 0859898..3e81c83 100644 --- a/src/reporter.ts +++ b/src/reporter.ts @@ -6,7 +6,7 @@ import ffmpeg from "fluent-ffmpeg"; import ffmpegInstaller from "@ffmpeg-installer/ffmpeg"; import { logger } from "./logger"; import { basePath } from "./utils"; -import { WorkerInfoStore } from "./fixture/workerInfo"; +import { WorkerInfo, WorkerInfoStore } from "./fixture/workerInfo"; class VideoDownloader implements Reporter { private downloadPromises: Promise[] = []; @@ -37,104 +37,186 @@ class VideoDownloader implements Reporter { const providerNameAnnotation = test.annotations.find( ({ type }) => type === "providerName", ); - if (sessionIdAnnotation && providerNameAnnotation) { + // Check if test ran on `device` or on `persistentDevice` + const isTestUsingDevice = sessionIdAnnotation && providerNameAnnotation; + if (isTestUsingDevice) { // This is a test that ran with the `device` fixture const sessionId = sessionIdAnnotation.description; const providerName = providerNameAnnotation.description; const provider = getProviderClass(providerName!); - const random = Math.floor(1000 + Math.random() * 9000); - const videoFileName = `${test.id}-${random}`; - const downloadPromise = new Promise((resolve) => { - provider - .downloadVideo(sessionId, basePath(), videoFileName) - .then( - (downloadedVideo: { path: string; contentType: string } | null) => { - if (!downloadedVideo) { - resolve(null); - return; - } - result.attachments.push({ - ...downloadedVideo, - name: "video", - }); - resolve(downloadedVideo); - }, - ); - }); - this.downloadPromises.push(downloadPromise); + this.downloadAndAttachDeviceVideo(test, result, provider, sessionId!); const otherAnnotations = test.annotations.filter( ({ type }) => type !== "sessionId" && type !== "providerName", ); test.annotations = otherAnnotations; } else { // This is a test that ran on `persistentDevice` fixture - const { workerIndex, startTime, duration } = result; - test.annotations.push({ - type: "workerInfo", - description: `Ran on worker #${workerIndex}.`, - }); + const { workerIndex, duration } = result; if (duration <= 0) { // Skipped tests return; } + test.annotations.push({ + type: "workerInfo", + description: `Ran on worker #${workerIndex}.`, + }); const expectedVideoPath = path.join( basePath(), `worker-${workerIndex}-video.mp4`, ); - const waitForWorkerToFinish = new Promise((resolve) => { - let maxIntervalTime = 60 * 60 * 1000; // 1 hour in ms - const interval = setInterval(async () => { - maxIntervalTime -= 500; - if (maxIntervalTime <= 0) { - clearInterval(interval); - logger.error("Timed out waiting for worker to finish"); - resolve(null); + // The `onTestEnd` is method is called before the worker ends and + // the worker's `endTime` is saved to disk. We add a 5 secs delay + // to prevent a harmful race condition. + const workerDownload = waitFiveSeconds() + .then(() => getWorkerInfo(workerIndex)) + .then(async (workerInfo) => { + if (!workerInfo) { + throw new Error(`Worker info not found for idx: ${workerIndex}`); } - if (fs.existsSync(expectedVideoPath)) { - clearInterval(interval); - const trimmedFileName = `worker-${workerIndex}-trimmed-${test.id}.mp4`; - const workerStart = await workerStartTime(workerIndex); - let pathToAttach = expectedVideoPath; - if (startTime.getTime() > workerStart.getTime()) { - // The startTime for the first test in the worker tends to be - // before worker (session) start time. This would have been manageable - // if the `duration` included the worker setup time, but the duration only - // covers the test method execution time. - // So in this case, we are not going to trim. - // TODO: We can use the startTime of the second test in the worker - const trimSkipPoint = - (startTime.getTime() - workerStart.getTime()) / 1000; - try { - pathToAttach = await trimVideo({ - originalVideoPath: expectedVideoPath, - startSecs: trimSkipPoint, - durationSecs: duration / 1000, - outputPath: trimmedFileName, - }); - } catch (e) { - logger.error("Failed to trim video:", e); - test.annotations.push({ - type: "videoError", - description: `Unable to trim video, attaching full video instead. Test starts at ${trimSkipPoint} secs.`, - }); - } + const { providerName, sessionId, endTime } = workerInfo; + if (!providerName || !sessionId) { + throw new Error( + `Provider name or session id not found for worker: ${workerIndex}`, + ); + } + if (!this.providerSupportsVideo(providerName)) { + return; // Nothing to do here + } + if (endTime) { + // This is the last test in the worker, so let's download the video + const provider = getProviderClass(providerName); + const downloaded: { + path: string; + contentType: string; + } | null = await provider.downloadVideo( + sessionId, + basePath(), + `worker-${workerIndex}-video`, + ); + if (!downloaded) { + return; } - result.attachments.push({ - path: pathToAttach, - contentType: "video/mp4", - name: "video", - }); - resolve(expectedVideoPath); + return this.trimAndAttachPersistentDeviceVideo( + test, + result, + downloaded.path, + ); + } else { + // This is an intermediate test in the worker, so let's wait for the + // video file to be found on disk. Once it is, we trim and attach it. + await waitFor(() => fs.existsSync(expectedVideoPath)); + return this.trimAndAttachPersistentDeviceVideo( + test, + result, + expectedVideoPath, + ); } - }, 500); - }); - this.downloadPromises.push(waitForWorkerToFinish); + }) + .catch((e) => { + logger.error("Failed to get worker end time:", e); + }); + this.downloadPromises.push(workerDownload); } } + async onEnd() { logger.log(`Triggered onEnd`); await Promise.allSettled(this.downloadPromises); } + + private async trimAndAttachPersistentDeviceVideo( + test: TestCase, + result: TestResult, + workerVideoPath: string, + ) { + const workerIdx = result.workerIndex; + const workerStart = await getWorkerStartTime(workerIdx); + let pathToAttach = workerVideoPath; + const testStart = result.startTime; + if (testStart.getTime() < workerStart.getTime()) { + // This is the first test for the worker + // The startTime for the first test in the worker tends to be + // before worker (session) start time. This would have been manageable + // if the `duration` included the worker setup time, but the duration only + // covers the test method execution time. + // So in this case, we are not going to trim. + // TODO: We can use the startTime of the second test in the worker + pathToAttach = workerVideoPath; + } else { + const trimSkipPoint = + (testStart.getTime() - workerStart.getTime()) / 1000; + const trimmedFileName = `worker-${workerIdx}-trimmed-${test.id}.mp4`; + try { + pathToAttach = await trimVideo({ + originalVideoPath: workerVideoPath, + startSecs: trimSkipPoint, + durationSecs: result.duration / 1000, + outputPath: trimmedFileName, + }); + } catch (e) { + logger.error("Failed to trim video:", e); + test.annotations.push({ + type: "videoError", + description: `Unable to trim video, attaching full video instead. Test starts at ${trimSkipPoint} secs.`, + }); + } + } + result.attachments.push({ + path: pathToAttach, + contentType: "video/mp4", + name: "video", + }); + } + + private downloadAndAttachDeviceVideo( + test: TestCase, + result: TestResult, + providerClass: any, + sessionId: string, + ) { + const videoFileName = `${test.id}`; + if (!providerClass.downloadVideo) { + return; + } + const downloadPromise = providerClass + .downloadVideo(sessionId, basePath(), videoFileName) + .then((downloadedVideo: { path: string; contentType: string } | null) => { + if (!downloadedVideo) { + return; + } + result.attachments.push({ + ...downloadedVideo, + name: "video", + }); + return downloadedVideo; + }); + this.downloadPromises.push(downloadPromise); + } + + private providerSupportsVideo(providerName: string) { + const provider = getProviderClass(providerName); + return !!provider.downloadVideo; + } +} + +function waitFor( + condition: () => boolean, + timeout: number = 60 * 60 * 1000, // 1 hour in ms +): Promise { + return new Promise((resolve, reject) => { + let interval: any; + const timeoutId = setTimeout(() => { + clearInterval(interval); + reject(new Error("Timed out waiting for condition")); + }, timeout); + interval = setInterval(() => { + if (condition()) { + clearInterval(interval); + clearTimeout(timeoutId); + resolve(); + } + }, 500); + }); } function trimVideo({ @@ -180,9 +262,17 @@ function trimVideo({ }); } -async function workerStartTime(idx: number): Promise { +async function getWorkerStartTime(idx: number): Promise { const workerInfoStore = new WorkerInfoStore(); return workerInfoStore.getWorkerStartTime(idx); } +async function getWorkerInfo(idx: number): Promise { + const workerInfoStore = new WorkerInfoStore(); + return workerInfoStore.getWorkerFromDisk(idx); +} + +const waitFiveSeconds = () => + new Promise((resolve) => setTimeout(resolve, 5000)); + export default VideoDownloader; diff --git a/src/types/index.ts b/src/types/index.ts index cb33f41..e480487 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -30,19 +30,6 @@ export interface DeviceProvider { */ getDevice(): Promise; - /** - * Downloads the video of the test. - * Currently, this functionality is supported only for BrowserStack. - * - * @param outputDir directory to save the video - * @param fileName name of the downloaded video file - * @returns - */ - downloadVideo?: ( - outputDir: string, - fileName: string, - ) => Promise<{ path: string; contentType: string } | null>; - /** * Updates test details and test status. *