From f1c0f96b75434ae1ebc9b3fdbfbc9d4afc2bbf4a Mon Sep 17 00:00:00 2001 From: coyotte508 Date: Fri, 21 Mar 2025 18:57:47 +0100 Subject: [PATCH 01/10] Add non-mocked test for symlink --- .../hub/src/lib/download-file-to-cache-dir.ts | 4 +- packages/hub/src/utils/symlink.spec.ts | 72 ++++--------------- packages/hub/src/utils/symlink.ts | 38 +++++----- 3 files changed, 37 insertions(+), 77 deletions(-) diff --git a/packages/hub/src/lib/download-file-to-cache-dir.ts b/packages/hub/src/lib/download-file-to-cache-dir.ts index 4059979a8e..2c2ff44c59 100644 --- a/packages/hub/src/lib/download-file-to-cache-dir.ts +++ b/packages/hub/src/lib/download-file-to-cache-dir.ts @@ -108,7 +108,7 @@ export async function downloadFileToCacheDir( // shortcut the download if needed if (await exists(blobPath)) { // create symlinks in snapshot folder to blob object - await createSymlink(blobPath, pointerPath); + await createSymlink({ sourcePath: blobPath, finalPath: pointerPath }); return pointerPath; } @@ -128,6 +128,6 @@ export async function downloadFileToCacheDir( // rename .incomplete file to expect blob await rename(incomplete, blobPath); // create symlinks in snapshot folder to blob object - await createSymlink(blobPath, pointerPath); + await createSymlink({ sourcePath: blobPath, finalPath: pointerPath }); return pointerPath; } diff --git a/packages/hub/src/utils/symlink.spec.ts b/packages/hub/src/utils/symlink.spec.ts index be2ad91d5c..5b3c4aa3f4 100644 --- a/packages/hub/src/utils/symlink.spec.ts +++ b/packages/hub/src/utils/symlink.spec.ts @@ -1,63 +1,21 @@ -import { describe, test, expect, vi, beforeEach } from "vitest"; -import * as fs from "node:fs/promises"; +import { describe, expect, it } from "vitest"; import { createSymlink } from "./symlink"; -import * as path from "node:path"; -import type { FileHandle } from "node:fs/promises"; - -vi.mock("node:fs/promises", () => ({ - rm: vi.fn(), - symlink: vi.fn(), - rename: vi.fn(), - copyFile: vi.fn(), - mkdir: vi.fn(), - mkdtemp: vi.fn(), - open: vi.fn(), -})); - -vi.mock("node:os", () => ({ - platform: vi.fn(), -})); - -describe("createSymlink", () => { - const src = "/path/to/src"; - const dst = "/path/to/dst"; - - beforeEach(() => { - vi.resetAllMocks(); - vi.mocked(fs.mkdtemp).mockImplementation(async (prefix) => `${prefix}/temp`); - vi.mocked(fs.open).mockResolvedValue({ close: vi.fn() } as unknown as FileHandle); - }); - - test("should remove existing destination", async () => { - await createSymlink(dst, src); - expect(fs.rm).toHaveBeenCalledWith(dst); - }); - - describe("symlink not supported (Windows)", () => { - beforeEach(() => { - vi.mocked(fs.symlink).mockRejectedValue(new Error("Symlink not supported")); +import { readFileSync, writeFileSync } from "node:fs"; +import { lstat } from "node:fs/promises"; + +describe("utils/symlink", () => { + it("should create a symlink", async () => { + writeFileSync("/tmp/test.txt", "hello world"); + await createSymlink({ + sourcePath: "/tmp/test.txt", + finalPath: "/tmp/test-symlink.txt", }); - test("should copyfile", async () => { - await createSymlink(dst, src); - expect(fs.copyFile).toHaveBeenCalledWith(path.resolve(dst), path.resolve(src)); - }); - - test("should rename file if new_blob is true", async () => { - await createSymlink(dst, src, true); - expect(fs.rename).toHaveBeenCalledWith(path.resolve(dst), path.resolve(src)); - }); - }); + const stats = await lstat("/tmp/test-symlink.txt"); + expect(stats.isSymbolicLink()).toBe(true); - describe("symlink supported", () => { - test("should symlink", async () => { - await createSymlink(dst, src); - expect(fs.symlink).toHaveBeenCalledWith(path.resolve(dst), path.resolve(src)); - }); - - test("should symlink if new_blob is true", async () => { - await createSymlink(dst, src, true); - expect(fs.symlink).toHaveBeenCalledWith(path.resolve(dst), path.resolve(src)); - }); + // Test file content + const content = readFileSync("/tmp/test-symlink.txt", "utf8"); + expect(content).toBe("hello world"); }); }); diff --git a/packages/hub/src/utils/symlink.ts b/packages/hub/src/utils/symlink.ts index 239b73cd18..a6d84d51d7 100644 --- a/packages/hub/src/utils/symlink.ts +++ b/packages/hub/src/utils/symlink.ts @@ -36,28 +36,30 @@ function expandUser(path: string): string { * having the actual file in `dst`. If it is a new file (`new_blob=True`), we move it to `dst`. If it is not a new file * (`new_blob=False`), we don't know if the blob file is already referenced elsewhere. To avoid breaking existing * cache, the file is duplicated on the disk. - * - * In case symlinks are not supported, a warning message is displayed to the user once when loading `huggingface_hub`. - * The warning message can be disabled with the `HF_HUB_DISABLE_SYMLINKS_WARNING` environment variable. */ -export async function createSymlink(dst: string, src: string, new_blob?: boolean): Promise { +export async function createSymlink(params: { + /** + * The path to the symlink. + */ + finalPath: string; + /** + * The path the symlink should point to. + */ + sourcePath: string; +}): Promise { + const abs_src = path.resolve(expandUser(params.sourcePath)); + const abs_dst = path.resolve(expandUser(params.finalPath)); + try { - await fs.rm(dst); - } catch (_e: unknown) { - /* empty */ + await fs.rm(abs_dst); + } catch { + // ignore } - const abs_src = path.resolve(expandUser(src)); - const abs_dst = path.resolve(expandUser(dst)); try { - await fs.symlink(abs_dst, abs_src); - } catch (_e: unknown) { - if (new_blob) { - console.info(`Symlink not supported. Moving file from ${abs_src} to ${abs_dst}`); - await fs.rename(abs_dst, abs_src); - } else { - console.info(`Symlink not supported. Copying file from ${abs_src} to ${abs_dst}`); - await fs.copyFile(abs_dst, abs_src); - } + await fs.symlink(abs_src, abs_dst); + } catch { + console.info(`Symlink not supported. Copying file from ${abs_src} to ${abs_dst}`); + await fs.copyFile(abs_src, abs_dst); } } From 1bb2f167266c48f70fb8352e37d148109795c281 Mon Sep 17 00:00:00 2001 From: coyotte508 Date: Fri, 21 Mar 2025 19:04:17 +0100 Subject: [PATCH 02/10] relative symlink --- packages/hub/src/utils/symlink.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/hub/src/utils/symlink.ts b/packages/hub/src/utils/symlink.ts index a6d84d51d7..17cedf66ad 100644 --- a/packages/hub/src/utils/symlink.ts +++ b/packages/hub/src/utils/symlink.ts @@ -57,7 +57,7 @@ export async function createSymlink(params: { } try { - await fs.symlink(abs_src, abs_dst); + await fs.symlink(path.relative(path.dirname(abs_dst), abs_src), abs_dst); } catch { console.info(`Symlink not supported. Copying file from ${abs_src} to ${abs_dst}`); await fs.copyFile(abs_src, abs_dst); From 609660e92bb448eed05a6cc60639dcedabf3e2a6 Mon Sep 17 00:00:00 2001 From: coyotte508 Date: Fri, 21 Mar 2025 19:07:36 +0100 Subject: [PATCH 03/10] one more test --- packages/hub/src/utils/symlink.spec.ts | 31 +++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/packages/hub/src/utils/symlink.spec.ts b/packages/hub/src/utils/symlink.spec.ts index 5b3c4aa3f4..7911802814 100644 --- a/packages/hub/src/utils/symlink.spec.ts +++ b/packages/hub/src/utils/symlink.spec.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "vitest"; import { createSymlink } from "./symlink"; import { readFileSync, writeFileSync } from "node:fs"; -import { lstat } from "node:fs/promises"; +import { lstat, rm } from "node:fs/promises"; describe("utils/symlink", () => { it("should create a symlink", async () => { @@ -17,5 +17,34 @@ describe("utils/symlink", () => { // Test file content const content = readFileSync("/tmp/test-symlink.txt", "utf8"); expect(content).toBe("hello world"); + + // Cleanup + await rm("/tmp/test-symlink.txt"); + await rm("/tmp/test.txt"); + }); + + it("should work when symlinking twice", async () => { + writeFileSync("/tmp/test.txt", "hello world"); + writeFileSync("/tmp/test2.txt", "hello world2"); + await createSymlink({ + sourcePath: "/tmp/test.txt", + finalPath: "/tmp/test-symlink.txt", + }); + await createSymlink({ + sourcePath: "/tmp/test2.txt", + finalPath: "/tmp/test-symlink.txt", + }); + + const stats = await lstat("/tmp/test-symlink.txt"); + expect(stats.isSymbolicLink()).toBe(true); + + // Test file content + const content = readFileSync("/tmp/test-symlink.txt", "utf8"); + expect(content).toBe("hello world2"); + + // Cleanup + await rm("/tmp/test-symlink.txt"); + await rm("/tmp/test.txt"); + await rm("/tmp/test2.txt"); }); }); From 3bcf70ddaa294dd701d731e581a2a3a0b327495f Mon Sep 17 00:00:00 2001 From: coyotte508 Date: Fri, 21 Mar 2025 19:09:14 +0100 Subject: [PATCH 04/10] comment out mocked tests --- .../lib/download-file-to-cache-dir.spec.ts | 588 +++++++++--------- 1 file changed, 295 insertions(+), 293 deletions(-) diff --git a/packages/hub/src/lib/download-file-to-cache-dir.spec.ts b/packages/hub/src/lib/download-file-to-cache-dir.spec.ts index 04ed7abd1e..71fc3f52e6 100644 --- a/packages/hub/src/lib/download-file-to-cache-dir.spec.ts +++ b/packages/hub/src/lib/download-file-to-cache-dir.spec.ts @@ -1,293 +1,295 @@ -import { expect, test, describe, vi, beforeEach } from "vitest"; -import type { RepoDesignation, RepoId } from "../types/public"; -import { dirname, join } from "node:path"; -import { lstat, mkdir, stat, symlink, writeFile, rename } from "node:fs/promises"; -import { pathsInfo } from "./paths-info"; -import type { Stats } from "node:fs"; -import { getHFHubCachePath, getRepoFolderName } from "./cache-management"; -import { toRepoId } from "../utils/toRepoId"; -import { downloadFileToCacheDir } from "./download-file-to-cache-dir"; -import { createSymlink } from "../utils/symlink"; - -vi.mock("node:fs/promises", () => ({ - writeFile: vi.fn(), - rename: vi.fn(), - symlink: vi.fn(), - lstat: vi.fn(), - mkdir: vi.fn(), - stat: vi.fn(), -})); - -vi.mock("./paths-info", () => ({ - pathsInfo: vi.fn(), -})); - -vi.mock("../utils/symlink", () => ({ - createSymlink: vi.fn(), -})); - -const DUMMY_REPO: RepoId = { - name: "hello-world", - type: "model", -}; - -const DUMMY_ETAG = "dummy-etag"; - -// utility test method to get blob file path -function _getBlobFile(params: { - repo: RepoDesignation; - etag: string; - cacheDir?: string; // default to {@link getHFHubCache} -}) { - return join(params.cacheDir ?? getHFHubCachePath(), getRepoFolderName(toRepoId(params.repo)), "blobs", params.etag); -} - -// utility test method to get snapshot file path -function _getSnapshotFile(params: { - repo: RepoDesignation; - path: string; - revision: string; - cacheDir?: string; // default to {@link getHFHubCache} -}) { - return join( - params.cacheDir ?? getHFHubCachePath(), - getRepoFolderName(toRepoId(params.repo)), - "snapshots", - params.revision, - params.path - ); -} - -describe("downloadFileToCacheDir", () => { - const fetchMock: typeof fetch = vi.fn(); - beforeEach(() => { - vi.resetAllMocks(); - // mock 200 request - vi.mocked(fetchMock).mockResolvedValue({ - status: 200, - ok: true, - body: "dummy-body", - } as unknown as Response); - - // prevent to use caching - vi.mocked(stat).mockRejectedValue(new Error("Do not exists")); - vi.mocked(lstat).mockRejectedValue(new Error("Do not exists")); - }); - - test("should throw an error if fileDownloadInfo return nothing", async () => { - await expect(async () => { - await downloadFileToCacheDir({ - repo: DUMMY_REPO, - path: "/README.md", - fetch: fetchMock, - }); - }).rejects.toThrowError("cannot get path info for /README.md"); - - expect(pathsInfo).toHaveBeenCalledWith( - expect.objectContaining({ - repo: DUMMY_REPO, - paths: ["/README.md"], - fetch: fetchMock, - }) - ); - }); - - test("existing symlinked and blob should not re-download it", async () => { - // ///snapshots/README.md - const expectPointer = _getSnapshotFile({ - repo: DUMMY_REPO, - path: "/README.md", - revision: "dd4bc8b21efa05ec961e3efc4ee5e3832a3679c7", - }); - // stat ensure a symlink and the pointed file exists - vi.mocked(stat).mockResolvedValue({} as Stats); // prevent default mocked reject - - const output = await downloadFileToCacheDir({ - repo: DUMMY_REPO, - path: "/README.md", - fetch: fetchMock, - revision: "dd4bc8b21efa05ec961e3efc4ee5e3832a3679c7", - }); - - expect(stat).toHaveBeenCalledOnce(); - // Get call argument for stat - const starArg = vi.mocked(stat).mock.calls[0][0]; - - expect(starArg).toBe(expectPointer); - expect(fetchMock).not.toHaveBeenCalledWith(); - - expect(output).toBe(expectPointer); - }); - - test("existing symlinked and blob with default revision should not re-download it", async () => { - // ///snapshots/README.md - const expectPointer = _getSnapshotFile({ - repo: DUMMY_REPO, - path: "/README.md", - revision: "main", - }); - // stat ensure a symlink and the pointed file exists - vi.mocked(stat).mockResolvedValue({} as Stats); // prevent default mocked reject - vi.mocked(lstat).mockResolvedValue({} as Stats); - vi.mocked(pathsInfo).mockResolvedValue([ - { - oid: DUMMY_ETAG, - size: 55, - path: "README.md", - type: "file", - lastCommit: { - date: new Date(), - id: "main", - title: "Commit msg", - }, - }, - ]); - - const output = await downloadFileToCacheDir({ - repo: DUMMY_REPO, - path: "/README.md", - fetch: fetchMock, - }); - - expect(stat).toHaveBeenCalledOnce(); - expect(symlink).not.toHaveBeenCalledOnce(); - // Get call argument for stat - const starArg = vi.mocked(stat).mock.calls[0][0]; - - expect(starArg).toBe(expectPointer); - expect(fetchMock).not.toHaveBeenCalledWith(); - - expect(output).toBe(expectPointer); - }); - - test("existing blob should only create the symlink", async () => { - // ///snapshots/README.md - const expectPointer = _getSnapshotFile({ - repo: DUMMY_REPO, - path: "/README.md", - revision: "dummy-commit-hash", - }); - // //blobs/ - const expectedBlob = _getBlobFile({ - repo: DUMMY_REPO, - etag: DUMMY_ETAG, - }); - - // mock existing blob only no symlink - vi.mocked(lstat).mockResolvedValue({} as Stats); - // mock pathsInfo resolve content - vi.mocked(pathsInfo).mockResolvedValue([ - { - oid: DUMMY_ETAG, - size: 55, - path: "README.md", - type: "file", - lastCommit: { - date: new Date(), - id: "dummy-commit-hash", - title: "Commit msg", - }, - }, - ]); - - const output = await downloadFileToCacheDir({ - repo: DUMMY_REPO, - path: "/README.md", - fetch: fetchMock, - }); - - // should have check for the blob - expect(lstat).toHaveBeenCalled(); - expect(vi.mocked(lstat).mock.calls[0][0]).toBe(expectedBlob); - - // symlink should have been created - expect(createSymlink).toHaveBeenCalledOnce(); - // no download done - expect(fetchMock).not.toHaveBeenCalled(); - - expect(output).toBe(expectPointer); - }); - - test("expect resolve value to be the pointer path of downloaded file", async () => { - // ///snapshots/README.md - const expectPointer = _getSnapshotFile({ - repo: DUMMY_REPO, - path: "/README.md", - revision: "dummy-commit-hash", - }); - // //blobs/ - const expectedBlob = _getBlobFile({ - repo: DUMMY_REPO, - etag: DUMMY_ETAG, - }); - - vi.mocked(pathsInfo).mockResolvedValue([ - { - oid: DUMMY_ETAG, - size: 55, - path: "README.md", - type: "file", - lastCommit: { - date: new Date(), - id: "dummy-commit-hash", - title: "Commit msg", - }, - }, - ]); - - const output = await downloadFileToCacheDir({ - repo: DUMMY_REPO, - path: "/README.md", - fetch: fetchMock, - }); - - // expect blobs and snapshots folder to have been mkdir - expect(vi.mocked(mkdir).mock.calls[0][0]).toBe(dirname(expectedBlob)); - expect(vi.mocked(mkdir).mock.calls[1][0]).toBe(dirname(expectPointer)); - - expect(output).toBe(expectPointer); - }); - - test("should write fetch response to blob", async () => { - // ///snapshots/README.md - const expectPointer = _getSnapshotFile({ - repo: DUMMY_REPO, - path: "/README.md", - revision: "dummy-commit-hash", - }); - // //blobs/ - const expectedBlob = _getBlobFile({ - repo: DUMMY_REPO, - etag: DUMMY_ETAG, - }); - - // mock pathsInfo resolve content - vi.mocked(pathsInfo).mockResolvedValue([ - { - oid: DUMMY_ETAG, - size: 55, - path: "README.md", - type: "file", - lastCommit: { - date: new Date(), - id: "dummy-commit-hash", - title: "Commit msg", - }, - }, - ]); - - await downloadFileToCacheDir({ - repo: DUMMY_REPO, - path: "/README.md", - fetch: fetchMock, - }); - - const incomplete = `${expectedBlob}.incomplete`; - // 1. should write fetch#response#body to incomplete file - expect(writeFile).toHaveBeenCalledWith(incomplete, "dummy-body"); - // 2. should rename the incomplete to the blob expected name - expect(rename).toHaveBeenCalledWith(incomplete, expectedBlob); - // 3. should create symlink pointing to blob - expect(createSymlink).toHaveBeenCalledWith(expectedBlob, expectPointer); - }); -}); +// Commented out, let's use non-mocked tests + +// import { expect, test, describe, vi, beforeEach } from "vitest"; +// import type { RepoDesignation, RepoId } from "../types/public"; +// import { dirname, join } from "node:path"; +// import { lstat, mkdir, stat, symlink, writeFile, rename } from "node:fs/promises"; +// import { pathsInfo } from "./paths-info"; +// import type { Stats } from "node:fs"; +// import { getHFHubCachePath, getRepoFolderName } from "./cache-management"; +// import { toRepoId } from "../utils/toRepoId"; +// import { downloadFileToCacheDir } from "./download-file-to-cache-dir"; +// import { createSymlink } from "../utils/symlink"; + +// vi.mock("node:fs/promises", () => ({ +// writeFile: vi.fn(), +// rename: vi.fn(), +// symlink: vi.fn(), +// lstat: vi.fn(), +// mkdir: vi.fn(), +// stat: vi.fn(), +// })); + +// vi.mock("./paths-info", () => ({ +// pathsInfo: vi.fn(), +// })); + +// vi.mock("../utils/symlink", () => ({ +// createSymlink: vi.fn(), +// })); + +// const DUMMY_REPO: RepoId = { +// name: "hello-world", +// type: "model", +// }; + +// const DUMMY_ETAG = "dummy-etag"; + +// // utility test method to get blob file path +// function _getBlobFile(params: { +// repo: RepoDesignation; +// etag: string; +// cacheDir?: string; // default to {@link getHFHubCache} +// }) { +// return join(params.cacheDir ?? getHFHubCachePath(), getRepoFolderName(toRepoId(params.repo)), "blobs", params.etag); +// } + +// // utility test method to get snapshot file path +// function _getSnapshotFile(params: { +// repo: RepoDesignation; +// path: string; +// revision: string; +// cacheDir?: string; // default to {@link getHFHubCache} +// }) { +// return join( +// params.cacheDir ?? getHFHubCachePath(), +// getRepoFolderName(toRepoId(params.repo)), +// "snapshots", +// params.revision, +// params.path +// ); +// } + +// describe("downloadFileToCacheDir", () => { +// const fetchMock: typeof fetch = vi.fn(); +// beforeEach(() => { +// vi.resetAllMocks(); +// // mock 200 request +// vi.mocked(fetchMock).mockResolvedValue({ +// status: 200, +// ok: true, +// body: "dummy-body", +// } as unknown as Response); + +// // prevent to use caching +// vi.mocked(stat).mockRejectedValue(new Error("Do not exists")); +// vi.mocked(lstat).mockRejectedValue(new Error("Do not exists")); +// }); + +// test("should throw an error if fileDownloadInfo return nothing", async () => { +// await expect(async () => { +// await downloadFileToCacheDir({ +// repo: DUMMY_REPO, +// path: "/README.md", +// fetch: fetchMock, +// }); +// }).rejects.toThrowError("cannot get path info for /README.md"); + +// expect(pathsInfo).toHaveBeenCalledWith( +// expect.objectContaining({ +// repo: DUMMY_REPO, +// paths: ["/README.md"], +// fetch: fetchMock, +// }) +// ); +// }); + +// test("existing symlinked and blob should not re-download it", async () => { +// // ///snapshots/README.md +// const expectPointer = _getSnapshotFile({ +// repo: DUMMY_REPO, +// path: "/README.md", +// revision: "dd4bc8b21efa05ec961e3efc4ee5e3832a3679c7", +// }); +// // stat ensure a symlink and the pointed file exists +// vi.mocked(stat).mockResolvedValue({} as Stats); // prevent default mocked reject + +// const output = await downloadFileToCacheDir({ +// repo: DUMMY_REPO, +// path: "/README.md", +// fetch: fetchMock, +// revision: "dd4bc8b21efa05ec961e3efc4ee5e3832a3679c7", +// }); + +// expect(stat).toHaveBeenCalledOnce(); +// // Get call argument for stat +// const starArg = vi.mocked(stat).mock.calls[0][0]; + +// expect(starArg).toBe(expectPointer); +// expect(fetchMock).not.toHaveBeenCalledWith(); + +// expect(output).toBe(expectPointer); +// }); + +// test("existing symlinked and blob with default revision should not re-download it", async () => { +// // ///snapshots/README.md +// const expectPointer = _getSnapshotFile({ +// repo: DUMMY_REPO, +// path: "/README.md", +// revision: "main", +// }); +// // stat ensure a symlink and the pointed file exists +// vi.mocked(stat).mockResolvedValue({} as Stats); // prevent default mocked reject +// vi.mocked(lstat).mockResolvedValue({} as Stats); +// vi.mocked(pathsInfo).mockResolvedValue([ +// { +// oid: DUMMY_ETAG, +// size: 55, +// path: "README.md", +// type: "file", +// lastCommit: { +// date: new Date(), +// id: "main", +// title: "Commit msg", +// }, +// }, +// ]); + +// const output = await downloadFileToCacheDir({ +// repo: DUMMY_REPO, +// path: "/README.md", +// fetch: fetchMock, +// }); + +// expect(stat).toHaveBeenCalledOnce(); +// expect(symlink).not.toHaveBeenCalledOnce(); +// // Get call argument for stat +// const starArg = vi.mocked(stat).mock.calls[0][0]; + +// expect(starArg).toBe(expectPointer); +// expect(fetchMock).not.toHaveBeenCalledWith(); + +// expect(output).toBe(expectPointer); +// }); + +// test("existing blob should only create the symlink", async () => { +// // ///snapshots/README.md +// const expectPointer = _getSnapshotFile({ +// repo: DUMMY_REPO, +// path: "/README.md", +// revision: "dummy-commit-hash", +// }); +// // //blobs/ +// const expectedBlob = _getBlobFile({ +// repo: DUMMY_REPO, +// etag: DUMMY_ETAG, +// }); + +// // mock existing blob only no symlink +// vi.mocked(lstat).mockResolvedValue({} as Stats); +// // mock pathsInfo resolve content +// vi.mocked(pathsInfo).mockResolvedValue([ +// { +// oid: DUMMY_ETAG, +// size: 55, +// path: "README.md", +// type: "file", +// lastCommit: { +// date: new Date(), +// id: "dummy-commit-hash", +// title: "Commit msg", +// }, +// }, +// ]); + +// const output = await downloadFileToCacheDir({ +// repo: DUMMY_REPO, +// path: "/README.md", +// fetch: fetchMock, +// }); + +// // should have check for the blob +// expect(lstat).toHaveBeenCalled(); +// expect(vi.mocked(lstat).mock.calls[0][0]).toBe(expectedBlob); + +// // symlink should have been created +// expect(createSymlink).toHaveBeenCalledOnce(); +// // no download done +// expect(fetchMock).not.toHaveBeenCalled(); + +// expect(output).toBe(expectPointer); +// }); + +// test("expect resolve value to be the pointer path of downloaded file", async () => { +// // ///snapshots/README.md +// const expectPointer = _getSnapshotFile({ +// repo: DUMMY_REPO, +// path: "/README.md", +// revision: "dummy-commit-hash", +// }); +// // //blobs/ +// const expectedBlob = _getBlobFile({ +// repo: DUMMY_REPO, +// etag: DUMMY_ETAG, +// }); + +// vi.mocked(pathsInfo).mockResolvedValue([ +// { +// oid: DUMMY_ETAG, +// size: 55, +// path: "README.md", +// type: "file", +// lastCommit: { +// date: new Date(), +// id: "dummy-commit-hash", +// title: "Commit msg", +// }, +// }, +// ]); + +// const output = await downloadFileToCacheDir({ +// repo: DUMMY_REPO, +// path: "/README.md", +// fetch: fetchMock, +// }); + +// // expect blobs and snapshots folder to have been mkdir +// expect(vi.mocked(mkdir).mock.calls[0][0]).toBe(dirname(expectedBlob)); +// expect(vi.mocked(mkdir).mock.calls[1][0]).toBe(dirname(expectPointer)); + +// expect(output).toBe(expectPointer); +// }); + +// test("should write fetch response to blob", async () => { +// // ///snapshots/README.md +// const expectPointer = _getSnapshotFile({ +// repo: DUMMY_REPO, +// path: "/README.md", +// revision: "dummy-commit-hash", +// }); +// // //blobs/ +// const expectedBlob = _getBlobFile({ +// repo: DUMMY_REPO, +// etag: DUMMY_ETAG, +// }); + +// // mock pathsInfo resolve content +// vi.mocked(pathsInfo).mockResolvedValue([ +// { +// oid: DUMMY_ETAG, +// size: 55, +// path: "README.md", +// type: "file", +// lastCommit: { +// date: new Date(), +// id: "dummy-commit-hash", +// title: "Commit msg", +// }, +// }, +// ]); + +// await downloadFileToCacheDir({ +// repo: DUMMY_REPO, +// path: "/README.md", +// fetch: fetchMock, +// }); + +// const incomplete = `${expectedBlob}.incomplete`; +// // 1. should write fetch#response#body to incomplete file +// expect(writeFile).toHaveBeenCalledWith(incomplete, "dummy-body"); +// // 2. should rename the incomplete to the blob expected name +// expect(rename).toHaveBeenCalledWith(incomplete, expectedBlob); +// // 3. should create symlink pointing to blob +// expect(createSymlink).toHaveBeenCalledWith(expectedBlob, expectPointer); +// }); +// }); From 531ea58299fcadb1226b1374f3cabf53a00e1c03 Mon Sep 17 00:00:00 2001 From: coyotte508 Date: Fri, 21 Mar 2025 23:14:21 +0100 Subject: [PATCH 05/10] add unit test for windows --- packages/hub/src/utils/symlink.spec.ts | 39 +++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/packages/hub/src/utils/symlink.spec.ts b/packages/hub/src/utils/symlink.spec.ts index 7911802814..970ce3417b 100644 --- a/packages/hub/src/utils/symlink.spec.ts +++ b/packages/hub/src/utils/symlink.spec.ts @@ -1,8 +1,24 @@ -import { describe, expect, it } from "vitest"; +/* eslint-disable @typescript-eslint/consistent-type-imports */ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import { describe, expect, it, vi } from "vitest"; import { createSymlink } from "./symlink"; import { readFileSync, writeFileSync } from "node:fs"; import { lstat, rm } from "node:fs/promises"; +let failSymlink = false; +vi.mock("node:fs/promises", async (importOriginal) => ({ + ...(await importOriginal()), + symlink: async (...args: any[]) => { + if (failSymlink) { + failSymlink = false; + throw new Error("Symlink not supported"); + } + + // @ts-expect-error - ignore + return (await importOriginal()).symlink(...args); + }, +})); + describe("utils/symlink", () => { it("should create a symlink", async () => { writeFileSync("/tmp/test.txt", "hello world"); @@ -47,4 +63,25 @@ describe("utils/symlink", () => { await rm("/tmp/test.txt"); await rm("/tmp/test2.txt"); }); + + it("should work when symlink doesn't work (windows)", async () => { + writeFileSync("/tmp/test.txt", "hello world"); + + failSymlink = true; + await createSymlink({ + sourcePath: "/tmp/test.txt", + finalPath: "/tmp/test-symlink.txt", + }); + + const stats = await lstat("/tmp/test-symlink.txt"); + expect(stats.isSymbolicLink()).toBe(false); + + // Test file content + const content = readFileSync("/tmp/test-symlink.txt", "utf8"); + expect(content).toBe("hello world"); + + // Cleanup + await rm("/tmp/test-symlink.txt"); + await rm("/tmp/test.txt"); + }); }); From 7840b339b35697b2cc42cd2c0a25383746d7db50 Mon Sep 17 00:00:00 2001 From: coyotte508 Date: Fri, 21 Mar 2025 23:16:38 +0100 Subject: [PATCH 06/10] maybe I went too far --- .../lib/download-file-to-cache-dir.spec.ts | 588 +++++++++--------- 1 file changed, 293 insertions(+), 295 deletions(-) diff --git a/packages/hub/src/lib/download-file-to-cache-dir.spec.ts b/packages/hub/src/lib/download-file-to-cache-dir.spec.ts index 71fc3f52e6..29d17f2870 100644 --- a/packages/hub/src/lib/download-file-to-cache-dir.spec.ts +++ b/packages/hub/src/lib/download-file-to-cache-dir.spec.ts @@ -1,295 +1,293 @@ -// Commented out, let's use non-mocked tests - -// import { expect, test, describe, vi, beforeEach } from "vitest"; -// import type { RepoDesignation, RepoId } from "../types/public"; -// import { dirname, join } from "node:path"; -// import { lstat, mkdir, stat, symlink, writeFile, rename } from "node:fs/promises"; -// import { pathsInfo } from "./paths-info"; -// import type { Stats } from "node:fs"; -// import { getHFHubCachePath, getRepoFolderName } from "./cache-management"; -// import { toRepoId } from "../utils/toRepoId"; -// import { downloadFileToCacheDir } from "./download-file-to-cache-dir"; -// import { createSymlink } from "../utils/symlink"; - -// vi.mock("node:fs/promises", () => ({ -// writeFile: vi.fn(), -// rename: vi.fn(), -// symlink: vi.fn(), -// lstat: vi.fn(), -// mkdir: vi.fn(), -// stat: vi.fn(), -// })); - -// vi.mock("./paths-info", () => ({ -// pathsInfo: vi.fn(), -// })); - -// vi.mock("../utils/symlink", () => ({ -// createSymlink: vi.fn(), -// })); - -// const DUMMY_REPO: RepoId = { -// name: "hello-world", -// type: "model", -// }; - -// const DUMMY_ETAG = "dummy-etag"; - -// // utility test method to get blob file path -// function _getBlobFile(params: { -// repo: RepoDesignation; -// etag: string; -// cacheDir?: string; // default to {@link getHFHubCache} -// }) { -// return join(params.cacheDir ?? getHFHubCachePath(), getRepoFolderName(toRepoId(params.repo)), "blobs", params.etag); -// } - -// // utility test method to get snapshot file path -// function _getSnapshotFile(params: { -// repo: RepoDesignation; -// path: string; -// revision: string; -// cacheDir?: string; // default to {@link getHFHubCache} -// }) { -// return join( -// params.cacheDir ?? getHFHubCachePath(), -// getRepoFolderName(toRepoId(params.repo)), -// "snapshots", -// params.revision, -// params.path -// ); -// } - -// describe("downloadFileToCacheDir", () => { -// const fetchMock: typeof fetch = vi.fn(); -// beforeEach(() => { -// vi.resetAllMocks(); -// // mock 200 request -// vi.mocked(fetchMock).mockResolvedValue({ -// status: 200, -// ok: true, -// body: "dummy-body", -// } as unknown as Response); - -// // prevent to use caching -// vi.mocked(stat).mockRejectedValue(new Error("Do not exists")); -// vi.mocked(lstat).mockRejectedValue(new Error("Do not exists")); -// }); - -// test("should throw an error if fileDownloadInfo return nothing", async () => { -// await expect(async () => { -// await downloadFileToCacheDir({ -// repo: DUMMY_REPO, -// path: "/README.md", -// fetch: fetchMock, -// }); -// }).rejects.toThrowError("cannot get path info for /README.md"); - -// expect(pathsInfo).toHaveBeenCalledWith( -// expect.objectContaining({ -// repo: DUMMY_REPO, -// paths: ["/README.md"], -// fetch: fetchMock, -// }) -// ); -// }); - -// test("existing symlinked and blob should not re-download it", async () => { -// // ///snapshots/README.md -// const expectPointer = _getSnapshotFile({ -// repo: DUMMY_REPO, -// path: "/README.md", -// revision: "dd4bc8b21efa05ec961e3efc4ee5e3832a3679c7", -// }); -// // stat ensure a symlink and the pointed file exists -// vi.mocked(stat).mockResolvedValue({} as Stats); // prevent default mocked reject - -// const output = await downloadFileToCacheDir({ -// repo: DUMMY_REPO, -// path: "/README.md", -// fetch: fetchMock, -// revision: "dd4bc8b21efa05ec961e3efc4ee5e3832a3679c7", -// }); - -// expect(stat).toHaveBeenCalledOnce(); -// // Get call argument for stat -// const starArg = vi.mocked(stat).mock.calls[0][0]; - -// expect(starArg).toBe(expectPointer); -// expect(fetchMock).not.toHaveBeenCalledWith(); - -// expect(output).toBe(expectPointer); -// }); - -// test("existing symlinked and blob with default revision should not re-download it", async () => { -// // ///snapshots/README.md -// const expectPointer = _getSnapshotFile({ -// repo: DUMMY_REPO, -// path: "/README.md", -// revision: "main", -// }); -// // stat ensure a symlink and the pointed file exists -// vi.mocked(stat).mockResolvedValue({} as Stats); // prevent default mocked reject -// vi.mocked(lstat).mockResolvedValue({} as Stats); -// vi.mocked(pathsInfo).mockResolvedValue([ -// { -// oid: DUMMY_ETAG, -// size: 55, -// path: "README.md", -// type: "file", -// lastCommit: { -// date: new Date(), -// id: "main", -// title: "Commit msg", -// }, -// }, -// ]); - -// const output = await downloadFileToCacheDir({ -// repo: DUMMY_REPO, -// path: "/README.md", -// fetch: fetchMock, -// }); - -// expect(stat).toHaveBeenCalledOnce(); -// expect(symlink).not.toHaveBeenCalledOnce(); -// // Get call argument for stat -// const starArg = vi.mocked(stat).mock.calls[0][0]; - -// expect(starArg).toBe(expectPointer); -// expect(fetchMock).not.toHaveBeenCalledWith(); - -// expect(output).toBe(expectPointer); -// }); - -// test("existing blob should only create the symlink", async () => { -// // ///snapshots/README.md -// const expectPointer = _getSnapshotFile({ -// repo: DUMMY_REPO, -// path: "/README.md", -// revision: "dummy-commit-hash", -// }); -// // //blobs/ -// const expectedBlob = _getBlobFile({ -// repo: DUMMY_REPO, -// etag: DUMMY_ETAG, -// }); - -// // mock existing blob only no symlink -// vi.mocked(lstat).mockResolvedValue({} as Stats); -// // mock pathsInfo resolve content -// vi.mocked(pathsInfo).mockResolvedValue([ -// { -// oid: DUMMY_ETAG, -// size: 55, -// path: "README.md", -// type: "file", -// lastCommit: { -// date: new Date(), -// id: "dummy-commit-hash", -// title: "Commit msg", -// }, -// }, -// ]); - -// const output = await downloadFileToCacheDir({ -// repo: DUMMY_REPO, -// path: "/README.md", -// fetch: fetchMock, -// }); - -// // should have check for the blob -// expect(lstat).toHaveBeenCalled(); -// expect(vi.mocked(lstat).mock.calls[0][0]).toBe(expectedBlob); - -// // symlink should have been created -// expect(createSymlink).toHaveBeenCalledOnce(); -// // no download done -// expect(fetchMock).not.toHaveBeenCalled(); - -// expect(output).toBe(expectPointer); -// }); - -// test("expect resolve value to be the pointer path of downloaded file", async () => { -// // ///snapshots/README.md -// const expectPointer = _getSnapshotFile({ -// repo: DUMMY_REPO, -// path: "/README.md", -// revision: "dummy-commit-hash", -// }); -// // //blobs/ -// const expectedBlob = _getBlobFile({ -// repo: DUMMY_REPO, -// etag: DUMMY_ETAG, -// }); - -// vi.mocked(pathsInfo).mockResolvedValue([ -// { -// oid: DUMMY_ETAG, -// size: 55, -// path: "README.md", -// type: "file", -// lastCommit: { -// date: new Date(), -// id: "dummy-commit-hash", -// title: "Commit msg", -// }, -// }, -// ]); - -// const output = await downloadFileToCacheDir({ -// repo: DUMMY_REPO, -// path: "/README.md", -// fetch: fetchMock, -// }); - -// // expect blobs and snapshots folder to have been mkdir -// expect(vi.mocked(mkdir).mock.calls[0][0]).toBe(dirname(expectedBlob)); -// expect(vi.mocked(mkdir).mock.calls[1][0]).toBe(dirname(expectPointer)); - -// expect(output).toBe(expectPointer); -// }); - -// test("should write fetch response to blob", async () => { -// // ///snapshots/README.md -// const expectPointer = _getSnapshotFile({ -// repo: DUMMY_REPO, -// path: "/README.md", -// revision: "dummy-commit-hash", -// }); -// // //blobs/ -// const expectedBlob = _getBlobFile({ -// repo: DUMMY_REPO, -// etag: DUMMY_ETAG, -// }); - -// // mock pathsInfo resolve content -// vi.mocked(pathsInfo).mockResolvedValue([ -// { -// oid: DUMMY_ETAG, -// size: 55, -// path: "README.md", -// type: "file", -// lastCommit: { -// date: new Date(), -// id: "dummy-commit-hash", -// title: "Commit msg", -// }, -// }, -// ]); - -// await downloadFileToCacheDir({ -// repo: DUMMY_REPO, -// path: "/README.md", -// fetch: fetchMock, -// }); - -// const incomplete = `${expectedBlob}.incomplete`; -// // 1. should write fetch#response#body to incomplete file -// expect(writeFile).toHaveBeenCalledWith(incomplete, "dummy-body"); -// // 2. should rename the incomplete to the blob expected name -// expect(rename).toHaveBeenCalledWith(incomplete, expectedBlob); -// // 3. should create symlink pointing to blob -// expect(createSymlink).toHaveBeenCalledWith(expectedBlob, expectPointer); -// }); -// }); +import { expect, test, describe, vi, beforeEach } from "vitest"; +import type { RepoDesignation, RepoId } from "../types/public"; +import { dirname, join } from "node:path"; +import { lstat, mkdir, stat, symlink, writeFile, rename } from "node:fs/promises"; +import { pathsInfo } from "./paths-info"; +import type { Stats } from "node:fs"; +import { getHFHubCachePath, getRepoFolderName } from "./cache-management"; +import { toRepoId } from "../utils/toRepoId"; +import { downloadFileToCacheDir } from "./download-file-to-cache-dir"; +import { createSymlink } from "../utils/symlink"; + +vi.mock("node:fs/promises", () => ({ + writeFile: vi.fn(), + rename: vi.fn(), + symlink: vi.fn(), + lstat: vi.fn(), + mkdir: vi.fn(), + stat: vi.fn(), +})); + +vi.mock("./paths-info", () => ({ + pathsInfo: vi.fn(), +})); + +vi.mock("../utils/symlink", () => ({ + createSymlink: vi.fn(), +})); + +const DUMMY_REPO: RepoId = { + name: "hello-world", + type: "model", +}; + +const DUMMY_ETAG = "dummy-etag"; + +// utility test method to get blob file path +function _getBlobFile(params: { + repo: RepoDesignation; + etag: string; + cacheDir?: string; // default to {@link getHFHubCache} +}) { + return join(params.cacheDir ?? getHFHubCachePath(), getRepoFolderName(toRepoId(params.repo)), "blobs", params.etag); +} + +// utility test method to get snapshot file path +function _getSnapshotFile(params: { + repo: RepoDesignation; + path: string; + revision: string; + cacheDir?: string; // default to {@link getHFHubCache} +}) { + return join( + params.cacheDir ?? getHFHubCachePath(), + getRepoFolderName(toRepoId(params.repo)), + "snapshots", + params.revision, + params.path + ); +} + +describe("downloadFileToCacheDir", () => { + const fetchMock: typeof fetch = vi.fn(); + beforeEach(() => { + vi.resetAllMocks(); + // mock 200 request + vi.mocked(fetchMock).mockResolvedValue({ + status: 200, + ok: true, + body: "dummy-body", + } as unknown as Response); + + // prevent to use caching + vi.mocked(stat).mockRejectedValue(new Error("Do not exists")); + vi.mocked(lstat).mockRejectedValue(new Error("Do not exists")); + }); + + test("should throw an error if fileDownloadInfo return nothing", async () => { + await expect(async () => { + await downloadFileToCacheDir({ + repo: DUMMY_REPO, + path: "/README.md", + fetch: fetchMock, + }); + }).rejects.toThrowError("cannot get path info for /README.md"); + + expect(pathsInfo).toHaveBeenCalledWith( + expect.objectContaining({ + repo: DUMMY_REPO, + paths: ["/README.md"], + fetch: fetchMock, + }) + ); + }); + + test("existing symlinked and blob should not re-download it", async () => { + // ///snapshots/README.md + const expectPointer = _getSnapshotFile({ + repo: DUMMY_REPO, + path: "/README.md", + revision: "dd4bc8b21efa05ec961e3efc4ee5e3832a3679c7", + }); + // stat ensure a symlink and the pointed file exists + vi.mocked(stat).mockResolvedValue({} as Stats); // prevent default mocked reject + + const output = await downloadFileToCacheDir({ + repo: DUMMY_REPO, + path: "/README.md", + fetch: fetchMock, + revision: "dd4bc8b21efa05ec961e3efc4ee5e3832a3679c7", + }); + + expect(stat).toHaveBeenCalledOnce(); + // Get call argument for stat + const starArg = vi.mocked(stat).mock.calls[0][0]; + + expect(starArg).toBe(expectPointer); + expect(fetchMock).not.toHaveBeenCalledWith(); + + expect(output).toBe(expectPointer); + }); + + test("existing symlinked and blob with default revision should not re-download it", async () => { + // ///snapshots/README.md + const expectPointer = _getSnapshotFile({ + repo: DUMMY_REPO, + path: "/README.md", + revision: "main", + }); + // stat ensure a symlink and the pointed file exists + vi.mocked(stat).mockResolvedValue({} as Stats); // prevent default mocked reject + vi.mocked(lstat).mockResolvedValue({} as Stats); + vi.mocked(pathsInfo).mockResolvedValue([ + { + oid: DUMMY_ETAG, + size: 55, + path: "README.md", + type: "file", + lastCommit: { + date: new Date(), + id: "main", + title: "Commit msg", + }, + }, + ]); + + const output = await downloadFileToCacheDir({ + repo: DUMMY_REPO, + path: "/README.md", + fetch: fetchMock, + }); + + expect(stat).toHaveBeenCalledOnce(); + expect(symlink).not.toHaveBeenCalledOnce(); + // Get call argument for stat + const starArg = vi.mocked(stat).mock.calls[0][0]; + + expect(starArg).toBe(expectPointer); + expect(fetchMock).not.toHaveBeenCalledWith(); + + expect(output).toBe(expectPointer); + }); + + test("existing blob should only create the symlink", async () => { + // ///snapshots/README.md + const expectPointer = _getSnapshotFile({ + repo: DUMMY_REPO, + path: "/README.md", + revision: "dummy-commit-hash", + }); + // //blobs/ + const expectedBlob = _getBlobFile({ + repo: DUMMY_REPO, + etag: DUMMY_ETAG, + }); + + // mock existing blob only no symlink + vi.mocked(lstat).mockResolvedValue({} as Stats); + // mock pathsInfo resolve content + vi.mocked(pathsInfo).mockResolvedValue([ + { + oid: DUMMY_ETAG, + size: 55, + path: "README.md", + type: "file", + lastCommit: { + date: new Date(), + id: "dummy-commit-hash", + title: "Commit msg", + }, + }, + ]); + + const output = await downloadFileToCacheDir({ + repo: DUMMY_REPO, + path: "/README.md", + fetch: fetchMock, + }); + + // should have check for the blob + expect(lstat).toHaveBeenCalled(); + expect(vi.mocked(lstat).mock.calls[0][0]).toBe(expectedBlob); + + // symlink should have been created + expect(createSymlink).toHaveBeenCalledOnce(); + // no download done + expect(fetchMock).not.toHaveBeenCalled(); + + expect(output).toBe(expectPointer); + }); + + test("expect resolve value to be the pointer path of downloaded file", async () => { + // ///snapshots/README.md + const expectPointer = _getSnapshotFile({ + repo: DUMMY_REPO, + path: "/README.md", + revision: "dummy-commit-hash", + }); + // //blobs/ + const expectedBlob = _getBlobFile({ + repo: DUMMY_REPO, + etag: DUMMY_ETAG, + }); + + vi.mocked(pathsInfo).mockResolvedValue([ + { + oid: DUMMY_ETAG, + size: 55, + path: "README.md", + type: "file", + lastCommit: { + date: new Date(), + id: "dummy-commit-hash", + title: "Commit msg", + }, + }, + ]); + + const output = await downloadFileToCacheDir({ + repo: DUMMY_REPO, + path: "/README.md", + fetch: fetchMock, + }); + + // expect blobs and snapshots folder to have been mkdir + expect(vi.mocked(mkdir).mock.calls[0][0]).toBe(dirname(expectedBlob)); + expect(vi.mocked(mkdir).mock.calls[1][0]).toBe(dirname(expectPointer)); + + expect(output).toBe(expectPointer); + }); + + test("should write fetch response to blob", async () => { + // ///snapshots/README.md + const expectPointer = _getSnapshotFile({ + repo: DUMMY_REPO, + path: "/README.md", + revision: "dummy-commit-hash", + }); + // //blobs/ + const expectedBlob = _getBlobFile({ + repo: DUMMY_REPO, + etag: DUMMY_ETAG, + }); + + // mock pathsInfo resolve content + vi.mocked(pathsInfo).mockResolvedValue([ + { + oid: DUMMY_ETAG, + size: 55, + path: "README.md", + type: "file", + lastCommit: { + date: new Date(), + id: "dummy-commit-hash", + title: "Commit msg", + }, + }, + ]); + + await downloadFileToCacheDir({ + repo: DUMMY_REPO, + path: "/README.md", + fetch: fetchMock, + }); + + const incomplete = `${expectedBlob}.incomplete`; + // 1. should write fetch#response#body to incomplete file + expect(writeFile).toHaveBeenCalledWith(incomplete, "dummy-body"); + // 2. should rename the incomplete to the blob expected name + expect(rename).toHaveBeenCalledWith(incomplete, expectedBlob); + // 3. should create symlink pointing to blob + expect(createSymlink).toHaveBeenCalledWith({ sourcePath: expectedBlob, finalPath: expectPointer }); + }); +}); From a384e10b3af8e499c84ba27b58244c455dc1275d Mon Sep 17 00:00:00 2001 From: coyotte508 Date: Fri, 21 Mar 2025 23:23:23 +0100 Subject: [PATCH 07/10] use os-spcific dir instead of /tmp --- packages/hub/src/utils/symlink.spec.ts | 52 +++++++++++++------------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/packages/hub/src/utils/symlink.spec.ts b/packages/hub/src/utils/symlink.spec.ts index 970ce3417b..159ae1f2a5 100644 --- a/packages/hub/src/utils/symlink.spec.ts +++ b/packages/hub/src/utils/symlink.spec.ts @@ -4,6 +4,8 @@ import { describe, expect, it, vi } from "vitest"; import { createSymlink } from "./symlink"; import { readFileSync, writeFileSync } from "node:fs"; import { lstat, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; let failSymlink = false; vi.mock("node:fs/promises", async (importOriginal) => ({ @@ -21,67 +23,67 @@ vi.mock("node:fs/promises", async (importOriginal) => ({ describe("utils/symlink", () => { it("should create a symlink", async () => { - writeFileSync("/tmp/test.txt", "hello world"); + writeFileSync(join(tmpdir(), "test.txt"), "hello world"); await createSymlink({ - sourcePath: "/tmp/test.txt", - finalPath: "/tmp/test-symlink.txt", + sourcePath: join(tmpdir(), "test.txt"), + finalPath: join(tmpdir(), "test-symlink.txt"), }); - const stats = await lstat("/tmp/test-symlink.txt"); + const stats = await lstat(join(tmpdir(), "test-symlink.txt")); expect(stats.isSymbolicLink()).toBe(true); // Test file content - const content = readFileSync("/tmp/test-symlink.txt", "utf8"); + const content = readFileSync(join(tmpdir(), "test-symlink.txt"), "utf8"); expect(content).toBe("hello world"); // Cleanup - await rm("/tmp/test-symlink.txt"); - await rm("/tmp/test.txt"); + await rm(join(tmpdir(), "test-symlink.txt")); + await rm(join(tmpdir(), "test.txt")); }); it("should work when symlinking twice", async () => { - writeFileSync("/tmp/test.txt", "hello world"); - writeFileSync("/tmp/test2.txt", "hello world2"); + writeFileSync(join(tmpdir(), "test.txt"), "hello world"); + writeFileSync(join(tmpdir(), "test2.txt"), "hello world2"); await createSymlink({ - sourcePath: "/tmp/test.txt", - finalPath: "/tmp/test-symlink.txt", + sourcePath: join(tmpdir(), "test.txt"), + finalPath: join(tmpdir(), "test-symlink.txt"), }); await createSymlink({ - sourcePath: "/tmp/test2.txt", - finalPath: "/tmp/test-symlink.txt", + sourcePath: join(tmpdir(), "test2.txt"), + finalPath: join(tmpdir(), "test-symlink.txt"), }); - const stats = await lstat("/tmp/test-symlink.txt"); + const stats = await lstat(join(tmpdir(), "test-symlink.txt")); expect(stats.isSymbolicLink()).toBe(true); // Test file content - const content = readFileSync("/tmp/test-symlink.txt", "utf8"); + const content = readFileSync(join(tmpdir(), "test-symlink.txt"), "utf8"); expect(content).toBe("hello world2"); // Cleanup - await rm("/tmp/test-symlink.txt"); - await rm("/tmp/test.txt"); - await rm("/tmp/test2.txt"); + await rm(join(tmpdir(), "test-symlink.txt")); + await rm(join(tmpdir(), "test.txt")); + await rm(join(tmpdir(), "test2.txt")); }); it("should work when symlink doesn't work (windows)", async () => { - writeFileSync("/tmp/test.txt", "hello world"); + writeFileSync(join(tmpdir(), "test.txt"), "hello world"); failSymlink = true; await createSymlink({ - sourcePath: "/tmp/test.txt", - finalPath: "/tmp/test-symlink.txt", + sourcePath: join(tmpdir(), "test.txt"), + finalPath: join(tmpdir(), "test-symlink.txt"), }); - const stats = await lstat("/tmp/test-symlink.txt"); + const stats = await lstat(join(tmpdir(), "test-symlink.txt")); expect(stats.isSymbolicLink()).toBe(false); // Test file content - const content = readFileSync("/tmp/test-symlink.txt", "utf8"); + const content = readFileSync(join(tmpdir(), "test-symlink.txt"), "utf8"); expect(content).toBe("hello world"); // Cleanup - await rm("/tmp/test-symlink.txt"); - await rm("/tmp/test.txt"); + await rm(join(tmpdir(), "test-symlink.txt")); + await rm(join(tmpdir(), "test.txt")); }); }); From 6cfb7380790dc0e43c6149ed60c7a827b31619a1 Mon Sep 17 00:00:00 2001 From: coyotte508 Date: Fri, 21 Mar 2025 23:24:39 +0100 Subject: [PATCH 08/10] maybe all tests work on windows --- packages/hub/src/utils/symlink.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/hub/src/utils/symlink.spec.ts b/packages/hub/src/utils/symlink.spec.ts index 159ae1f2a5..56edaf4f33 100644 --- a/packages/hub/src/utils/symlink.spec.ts +++ b/packages/hub/src/utils/symlink.spec.ts @@ -30,7 +30,7 @@ describe("utils/symlink", () => { }); const stats = await lstat(join(tmpdir(), "test-symlink.txt")); - expect(stats.isSymbolicLink()).toBe(true); + expect(stats.isSymbolicLink()).toBe(process.platform !== "win32"); // Test file content const content = readFileSync(join(tmpdir(), "test-symlink.txt"), "utf8"); @@ -54,7 +54,7 @@ describe("utils/symlink", () => { }); const stats = await lstat(join(tmpdir(), "test-symlink.txt")); - expect(stats.isSymbolicLink()).toBe(true); + expect(stats.isSymbolicLink()).toBe(process.platform !== "win32"); // Test file content const content = readFileSync(join(tmpdir(), "test-symlink.txt"), "utf8"); From 0f7978089a336050f60dc0cb19d4d008beabbbb2 Mon Sep 17 00:00:00 2001 From: coyotte508 Date: Fri, 21 Mar 2025 23:28:13 +0100 Subject: [PATCH 09/10] increase timeout --- packages/hub/vitest.config.mts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/hub/vitest.config.mts b/packages/hub/vitest.config.mts index 88747caaed..cc84b9c512 100644 --- a/packages/hub/vitest.config.mts +++ b/packages/hub/vitest.config.mts @@ -2,6 +2,6 @@ import { defineConfig } from "vitest/config"; export default defineConfig({ test: { - testTimeout: 30_000, + testTimeout: 60_000, }, }); From 9c99a87dfd27dd35a2c7650773d5fd668142ffd7 Mon Sep 17 00:00:00 2001 From: coyotte508 Date: Fri, 21 Mar 2025 23:28:46 +0100 Subject: [PATCH 10/10] remove internal loggin --- packages/hub/src/utils/XetBlob.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/hub/src/utils/XetBlob.spec.ts b/packages/hub/src/utils/XetBlob.spec.ts index 3539ab3473..008e2294d2 100644 --- a/packages/hub/src/utils/XetBlob.spec.ts +++ b/packages/hub/src/utils/XetBlob.spec.ts @@ -98,7 +98,7 @@ describe("XetBlob", () => { } return fetch(url, opts); }, - internalLogging: true, + // internalLogging: true, }); const xetDownload = await blob.slice(0, 200_000).arrayBuffer(); @@ -124,7 +124,7 @@ describe("XetBlob", () => { }, hash: "7b3b6d07673a88cf467e67c1f7edef1a8c268cbf66e9dd9b0366322d4ab56d9b", size: 5_234_139_343, - internalLogging: true, + // internalLogging: true, }); const xetDownload = await blob.slice(10_000_000, 10_100_000).arrayBuffer();