davidl/MyTube
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

test: Add file_location test and mock settings in ytdlpVideo

Browse Source
This commit is contained in:
Peifan Li
2026-01-03 23:58:13 -05:00
parent d99a210174
commit a5e82b9e81
5 changed files with 235 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
backend/src/__tests__/services/downloaders/YtDlpDownloader.safari.test.ts
View File

@@ -23,6 +23,7 @@ vi.mock('../../../services/storageService', () => ({
saveVideo: vi.fn(),
getVideoBySourceUrl: vi.fn(),
updateVideo: vi.fn(),
getSettings: vi.fn().mockReturnValue({}),
}));
// Mock fs-extra - define mockWriter inside the factory

168
backend/src/__tests__/services/downloaders/file_location.test.ts Normal file
View File

@@ -0,0 +1,168 @@
import path from 'path';
import { beforeEach, describe, expect, it, vi } from 'vitest';
// Use vi.hoisted to ensure mocks are available for vi.mock factory
const mocks = vi.hoisted(() => {
return {
executeYtDlpSpawn: vi.fn(),
executeYtDlpJson: vi.fn(),
getUserYtDlpConfig: vi.fn(),
getSettings: vi.fn(),
readdirSync: vi.fn(),
readFileSync: vi.fn(),
writeFileSync: vi.fn(),
unlinkSync: vi.fn(),
remove: vi.fn(),
};
});
// Setup default return values in the factory or beforeEach
mocks.executeYtDlpJson.mockResolvedValue({
title: 'Test Video',
uploader: 'Test Author',
upload_date: '20230101',
thumbnail: 'http://example.com/thumb.jpg',
extractor: 'youtube'
});
mocks.getUserYtDlpConfig.mockReturnValue({});
mocks.getSettings.mockReturnValue({});
mocks.readdirSync.mockReturnValue([]);
mocks.readFileSync.mockReturnValue('WEBVTT');
vi.mock('../../../config/paths', () => ({
VIDEOS_DIR: '/mock/videos',
IMAGES_DIR: '/mock/images',
SUBTITLES_DIR: '/mock/subtitles',
}));
vi.mock('../../../utils/ytDlpUtils', () => ({
executeYtDlpSpawn: (...args: any[]) => mocks.executeYtDlpSpawn(...args),
executeYtDlpJson: (...args: any[]) => mocks.executeYtDlpJson(...args),
getUserYtDlpConfig: (...args: any[]) => mocks.getUserYtDlpConfig(...args),
getNetworkConfigFromUserConfig: () => ({})
}));
vi.mock('../../../services/storageService', () => ({
updateActiveDownload: vi.fn(),
saveVideo: vi.fn(),
getVideoBySourceUrl: vi.fn(),
updateVideo: vi.fn(),
getSettings: () => mocks.getSettings(),
}));
// Mock processSubtitles to verify it receives correct arguments
// We need to access the actual implementation in logic but for this test checking arguments might be enough
// However, the real test is seeing if paths are correct in downloadVideo
// And we want to test processSubtitles logic too.
// Let's mock fs-extra completely
vi.mock('fs-extra', () => {
return {
default: {
pathExists: vi.fn().mockResolvedValue(false),
ensureDirSync: vi.fn(),
existsSync: vi.fn().mockReturnValue(false),
createWriteStream: vi.fn().mockReturnValue({
on: (event: string, cb: any) => {
if (event === 'finish') cb();
return { on: vi.fn() };
}
}),
readdirSync: (...args: any[]) => mocks.readdirSync(...args),
readFileSync: (...args: any[]) => mocks.readFileSync(...args),
writeFileSync: (...args: any[]) => mocks.writeFileSync(...args),
unlinkSync: (...args: any[]) => mocks.unlinkSync(...args),
remove: (...args: any[]) => mocks.remove(...args),
statSync: vi.fn().mockReturnValue({ size: 1000 }),
}
};
});
vi.mock('axios', () => ({
default: vi.fn().mockResolvedValue({
data: {
pipe: (writer: any) => {
// Simulate write finish if writer has on method
if (writer.on) {
// Find and call finish handler manually if needed
// But strictly relying on the createWriteStream mock above handling it
}
}
}
})
}));
vi.mock('../../../services/metadataService', () => ({
getVideoDuration: vi.fn().mockResolvedValue(null),
}));
vi.mock('../../../utils/downloadUtils', () => ({
isDownloadActive: vi.fn().mockReturnValue(true), // Always active
isCancellationError: vi.fn().mockReturnValue(false),
cleanupSubtitleFiles: vi.fn(),
cleanupVideoArtifacts: vi.fn(),
}));
// Import the modules under test
import { processSubtitles } from '../../../services/downloaders/ytdlp/ytdlpSubtitle';
describe('File Location Logic', () => {
beforeEach(() => {
vi.clearAllMocks();
mocks.executeYtDlpSpawn.mockReturnValue({
stdout: { on: vi.fn() },
kill: vi.fn(),
then: (resolve: any) => resolve()
});
mocks.readdirSync.mockReturnValue([]);
// Reset default mock implementations if needed, but they are set on the object so clearer to set logic in test
});
// describe('downloadVideo', () => {});
describe('processSubtitles', () => {
it('should move subtitles to SUBTITLES_DIR by default', async () => {
const baseFilename = 'video_123';
mocks.readdirSync.mockReturnValue(['video_123.en.vtt']);
mocks.readFileSync.mockReturnValue('WEBVTT');
await processSubtitles(baseFilename, 'download_id', false);
expect(mocks.writeFileSync).toHaveBeenCalledWith(
path.join('/mock/subtitles', 'video_123.en.vtt'),
expect.any(String),
'utf-8'
);
expect(mocks.unlinkSync).toHaveBeenCalledWith(
path.join('/mock/videos', 'video_123.en.vtt')
);
});
it('should keep subtitles in VIDEOS_DIR if moveSubtitlesToVideoFolder is true', async () => {
const baseFilename = 'video_123';
mocks.readdirSync.mockReturnValue(['video_123.en.vtt']);
mocks.readFileSync.mockReturnValue('WEBVTT');
await processSubtitles(baseFilename, 'download_id', true);
// Expect destination to be VIDEOS_DIR
expect(mocks.writeFileSync).toHaveBeenCalledWith(
path.join('/mock/videos', 'video_123.en.vtt'),
expect.any(String),
'utf-8'
);
// source and dest are technically same dir (but maybe different filenames if lang was parsed differently?)
// In typical case: source = /videos/video_123.en.vtt, dest = /videos/video_123.en.vtt
// Code says: if (sourceSubPath !== destSubPath) unlinkSync
// Using mock path.join, let's trace:
// source = /mock/videos/video_123.en.vtt
// dest = /mock/videos/video_123.en.vtt
// So unlinkSync should NOT be called
expect(mocks.unlinkSync).not.toHaveBeenCalled();
});
});
});

28
backend/src/services/downloaders/ytdlp/ytdlpSubtitle.ts
View File

@@ -32,7 +32,8 @@ class YtDlpDownloaderHelper extends BaseDownloader {
*/
export async function processSubtitles(
baseFilename: string,
downloadId?: string
downloadId?: string,
moveSubtitlesToVideoFolder: boolean = false
): Promise<Array<{ language: string; filename: string; path: string }>> {
const subtitles: Array<{ language: string; filename: string; path: string }> =
[];
@@ -64,11 +65,20 @@ export async function processSubtitles(
);
const language = match ? match[1] : "unknown";
// Move subtitle to subtitles directory
// Move subtitle to subtitles directory or keep in video directory if requested
const sourceSubPath = path.join(VIDEOS_DIR, subtitleFile);
const destSubFilename = `${baseFilename}.${language}.vtt`;
const destSubPath = path.join(SUBTITLES_DIR, destSubFilename);
let destSubPath: string;
let webPath: string;
if (moveSubtitlesToVideoFolder) {
destSubPath = path.join(VIDEOS_DIR, destSubFilename);
webPath = `/videos/${destSubFilename}`;
} else {
destSubPath = path.join(SUBTITLES_DIR, destSubFilename);
webPath = `/subtitles/${destSubFilename}`;
}
// Read VTT file and fix alignment for centering
let vttContent = fs.readFileSync(sourceSubPath, "utf-8");
// Replace align:start with align:middle for centered subtitles
@@ -79,8 +89,14 @@ export async function processSubtitles(
// Write cleaned VTT to destination
fs.writeFileSync(destSubPath, vttContent, "utf-8");
// Remove original file
fs.unlinkSync(sourceSubPath);
// Remove original file if we moved it (if dest is different from source)
// If moveSubtitlesToVideoFolder is true, destSubPath might be same as sourceSubPath
// but with different name (e.g. video_uuid.en.vtt vs video_uuid.vtt)
// Actually source is usually video_uuid.en.vtt (from yt-dlp) and dest is video_uuid.en.vtt
// So if names are same and dir is same, we're just overwriting in place, which is fine
if (sourceSubPath !== destSubPath) {
fs.unlinkSync(sourceSubPath);
}
logger.info(
`Processed and moved subtitle ${subtitleFile} to ${destSubPath}`
@@ -89,7 +105,7 @@ export async function processSubtitles(
subtitles.push({
language,
filename: destSubFilename,
path: `/subtitles/${destSubFilename}`,
path: webPath,
});
}
} catch (subtitleError) {

50
backend/src/services/downloaders/ytdlp/ytdlpVideo.ts
View File

@@ -2,17 +2,17 @@ import fs from "fs-extra";
import path from "path";
import { IMAGES_DIR, VIDEOS_DIR } from "../../../config/paths";
import {
cleanupSubtitleFiles,
cleanupVideoArtifacts,
cleanupSubtitleFiles,
cleanupVideoArtifacts,
} from "../../../utils/downloadUtils";
import { formatVideoFilename } from "../../../utils/helpers";
import { logger } from "../../../utils/logger";
import { ProgressTracker } from "../../../utils/progressTracker";
import {
executeYtDlpJson,
executeYtDlpSpawn,
getNetworkConfigFromUserConfig,
getUserYtDlpConfig,
executeYtDlpJson,
executeYtDlpSpawn,
getNetworkConfigFromUserConfig,
getUserYtDlpConfig,
} from "../../../utils/ytDlpUtils";
import * as storageService from "../../storageService";
import { Video } from "../../storageService";
@@ -136,8 +136,16 @@ export async function downloadVideo(
finalThumbnailFilename = newThumbnailFilename;
// Update paths
const settings = storageService.getSettings();
const moveThumbnailsToVideoFolder =
settings.moveThumbnailsToVideoFolder || false;
const moveSubtitlesToVideoFolder =
settings.moveSubtitlesToVideoFolder || false;
const newVideoPath = path.join(VIDEOS_DIR, finalVideoFilename);
const newThumbnailPath = path.join(IMAGES_DIR, finalThumbnailFilename);
const newThumbnailPath = moveThumbnailsToVideoFolder
? path.join(VIDEOS_DIR, finalThumbnailFilename)
: path.join(IMAGES_DIR, finalThumbnailFilename);
logger.info("Preparing video download path:", newVideoPath);
@@ -203,7 +211,13 @@ export async function downloadVideo(
// Clean up partial files
logger.info("Cleaning up partial files...");
await cleanupVideoArtifacts(newSafeBaseFilename);
await cleanupVideoArtifacts(newSafeBaseFilename, IMAGES_DIR);
// Use fresh cleanup based on settings
const currentSettings = storageService.getSettings();
if (!currentSettings.moveThumbnailsToVideoFolder) {
await cleanupVideoArtifacts(newSafeBaseFilename, IMAGES_DIR);
}
if (fs.existsSync(newThumbnailPath)) {
await fs.remove(newThumbnailPath);
}
@@ -293,13 +307,21 @@ export async function downloadVideo(
}
// Process subtitle files
subtitles = await processSubtitles(newSafeBaseFilename, downloadId);
subtitles = await processSubtitles(
newSafeBaseFilename,
downloadId,
moveSubtitlesToVideoFolder
);
} catch (error) {
logger.error("Error in download process:", error);
throw error;
}
// Create metadata for the video
const settings = storageService.getSettings();
const moveThumbnailsToVideoFolder =
settings.moveThumbnailsToVideoFolder || false;
const videoData: Video = {
id: timestamp.toString(),
title: videoTitle || "Video",
@@ -312,7 +334,11 @@ export async function downloadVideo(
thumbnailFilename: thumbnailSaved ? finalThumbnailFilename : undefined,
thumbnailUrl: thumbnailUrl || undefined,
videoPath: `/videos/${finalVideoFilename}`,
thumbnailPath: thumbnailSaved ? `/images/${finalThumbnailFilename}` : null,
thumbnailPath: thumbnailSaved
? moveThumbnailsToVideoFolder
? `/videos/${finalThumbnailFilename}`
: `/images/${finalThumbnailFilename}`
: null,
subtitles: subtitles.length > 0 ? subtitles : undefined,
duration: undefined, // Will be populated below
channelUrl: channelUrl || undefined,
@@ -367,7 +393,9 @@ export async function downloadVideo(
? finalThumbnailFilename
: existingVideo.thumbnailFilename,
thumbnailPath: thumbnailSaved
? `/images/${finalThumbnailFilename}`
? moveThumbnailsToVideoFolder
? `/videos/${finalThumbnailFilename}`
: `/images/${finalThumbnailFilename}`
: existingVideo.thumbnailPath,
duration: videoData.duration,
fileSize: videoData.fileSize,

6
frontend/src/pages/SettingsPage.tsx
View File

@@ -92,7 +92,7 @@ const SettingsPage: React.FC = () => {
const isSticky = useStickyButton(observerTarget);
// Fetch settings
const { data: settingsData } = useQuery({
const { data: settingsData, refetch } = useQuery({
queryKey: ['settings'],
queryFn: async () => {
const response = await axios.get(`${API_URL}/settings`);
@@ -100,6 +100,10 @@ const SettingsPage: React.FC = () => {
}
});
useEffect(() => {
refetch();
}, [refetch]);
useEffect(() => {
if (settingsData) {
const newSettings = {