From 45649ae174579aa74031c74286a9326fccbe0049 Mon Sep 17 00:00:00 2001 From: John Harrington <84741727+harr1424@users.noreply.github.com> Date: Sun, 21 Sep 2025 18:22:00 -0700 Subject: [PATCH] add/update unit tests to import.service.ts and metadata.rs --- .../src/metadata.rs | 101 +++++++ .../src/services/import.service.spec.ts | 251 ++++++++++++------ libs/importer/src/util.spec.ts | 68 ----- 3 files changed, 269 insertions(+), 151 deletions(-) delete mode 100644 libs/importer/src/util.spec.ts diff --git a/apps/desktop/desktop_native/bitwarden_chromium_importer/src/metadata.rs b/apps/desktop/desktop_native/bitwarden_chromium_importer/src/metadata.rs index f1fbb860a7e..6868c09f2c4 100644 --- a/apps/desktop/desktop_native/bitwarden_chromium_importer/src/metadata.rs +++ b/apps/desktop/desktop_native/bitwarden_chromium_importer/src/metadata.rs @@ -51,3 +51,104 @@ pub fn get_supported_importers() -> HashMap { map } + +/* + Tests are cfg-gated based upon OS, and must be compiled/run on each OS for full coverage +*/ +#[cfg(test)] +mod tests { + use super::*; + use std::collections::HashSet; + + fn map_keys(map: &HashMap) -> HashSet { + map.keys().cloned().collect() + } + + fn get_loaders(map: &HashMap, id: &str) -> HashSet<&'static str> { + map.get(id) + .map(|m| m.loaders.iter().copied().collect::>()) + .unwrap_or_default() + } + + #[test] + fn returns_all_known_importers() { + let map = get_supported_importers(); + + let expected: HashSet = HashSet::from([ + "chromecsv".to_string(), + "chromiumcsv".to_string(), + "bravecsv".to_string(), + "operacsv".to_string(), + "vivaldicsv".to_string(), + "edgecsv".to_string(), + ]); + assert_eq!(map.len(), expected.len()); + assert_eq!(map_keys(&map), expected); + + for (key, meta) in map.iter() { + assert_eq!(&meta.id, key); + assert_eq!(meta.instructions, "chromium"); + assert!(meta.loaders.iter().any(|l| *l == "file")); + } + } + + #[cfg(target_os = "macos")] + #[test] + fn macos_specific_loaders_match_const_array() { + let map = get_supported_importers(); + let ids = [ + "chromecsv", + "chromiumcsv", + "bravecsv", + "operacsv", + "vivaldicsv", + "edgecsv", + ]; + for id in ids { + let loaders = get_loaders(&map, id); + assert!(loaders.contains("file")); + assert!(loaders.contains("chromium"), "missing chromium for {id}"); + } + } + + #[cfg(target_os = "linux")] + #[test] + fn linux_specific_loaders_match_const_array() { + let map = get_supported_importers(); + let with_chromium = ["chromecsv", "chromiumcsv", "bravecsv", "operacsv"]; + let without_chromium = ["vivaldicsv", "edgecsv"]; + + for id in with_chromium { + let loaders = get_loaders(&map, id); + assert!(loaders.contains("file")); + assert!(loaders.contains("chromium"), "missing chromium for {id}"); + } + + for id in without_chromium { + let loaders = get_loaders(&map, id); + assert!(loaders.contains("file")); + assert!(!loaders.contains("chromium"), "unexpected chromium support for {id}"); + } + } + + #[cfg(target_os = "windows")] + #[test] + fn windows_specific_loaders_match_const_array() { + let map = get_supported_importers(); + let with_chromium = ["chromiumcsv", "edgecsv", "operacsv", "vivaldicsv"]; + let without_chromium = ["chromecsv", "bravecsv"]; + + for id in with_chromium { + let loaders = get_loaders(&map, id); + assert!(loaders.contains("file")); + assert!(loaders.contains("chromium"), "missing chromium for {id}"); + } + + for id in without_chromium { + let loaders = get_loaders(&map, id); + assert!(loaders.contains("file")); + assert!(!loaders.contains("chromium"), "unexpected chromium support for {id}"); + } + } +} + diff --git a/libs/importer/src/services/import.service.spec.ts b/libs/importer/src/services/import.service.spec.ts index 3e368c16ec1..b42494ab115 100644 --- a/libs/importer/src/services/import.service.spec.ts +++ b/libs/importer/src/services/import.service.spec.ts @@ -1,7 +1,5 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { mock, MockProxy } from "jest-mock-extended"; -import { BehaviorSubject } from "rxjs"; +import { BehaviorSubject, Subject, firstValueFrom } from "rxjs"; // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. // eslint-disable-next-line no-restricted-imports @@ -25,8 +23,8 @@ import { KeyService } from "@bitwarden/key-management"; import { BitwardenPasswordProtectedImporter } from "../importers/bitwarden/bitwarden-password-protected-importer"; import { Importer } from "../importers/importer"; -// import { ImporterMetadata, Instructions, Loader } from "../metadata"; -// import { ImportType } from "../models"; +import type { ImporterMetadata } from "../metadata"; +import { ImportType } from "../models"; import { ImportResult } from "../models/import-result"; import { ImportApiServiceAbstraction } from "./import-api.service.abstraction"; @@ -269,31 +267,87 @@ describe("ImportService", () => { }); }); - // TODO Move capability/metadata tests to Rust unit tests and remove this disable. - /* describe("metadata$", () => { - let featureFlagSubject: BehaviorSubject; - let typeSubject: Subject; - let mockLogger: { debug: jest.Mock }; + describe("metadata$", () => { + // Helper to build a native-like metadata map and re-import ImportService after mocking + async function createServiceWithNativeMap( + nativeMap: Partial< + Record + >, + ) { + jest.resetModules(); - beforeEach(() => { - featureFlagSubject = new BehaviorSubject(false); - typeSubject = new Subject(); - mockLogger = { debug: jest.fn() }; + jest.doMock("../metadata", () => { + const data = jest.requireActual("../metadata/data"); + const availability = jest.requireActual("../metadata/availability"); + const { Loader, Instructions } = data; + const Importers = Object.freeze( + Object.fromEntries( + Object.entries(nativeMap).map(([id, meta]) => { + const loaders = meta.loaders.map((l) => (Loader as any)[l]); + const instructions = meta.instructions + ? (Instructions as any)[meta.instructions] + : undefined; + return [ + id, + { + type: id, + loaders, + ...(instructions ? { instructions } : {}), + }, + ]; + }), + ), + ); + return { + Loader, + Instructions, + LoaderAvailability: availability.LoaderAvailability, + Importers, + }; + }); + + jest.doMock("../util", () => { + const { Loader } = jest.requireActual("../metadata/data"); + const { LoaderAvailability } = jest.requireActual("../metadata/availability"); + const availableLoaders = ( + type: ImportType, + client: import("@bitwarden/client-type").ClientType, + ) => { + const entry = nativeMap[type]; + if (!entry) { + return undefined; + } + const loaders = entry.loaders.map( + (l) => (Loader as any)[l], + ) as import("../metadata/types").DataLoader[]; + return loaders.filter((loader: import("../metadata/types").DataLoader) => + LoaderAvailability[loader]?.includes(client), + ); + }; + return { availableLoaders }; + }); const configService = mock(); + const featureFlagSubject = new BehaviorSubject(false); configService.getFeatureFlag$.mockReturnValue(featureFlagSubject); const environment = mock(); environment.getClientType.mockReturnValue(ClientType.Desktop); + const mockLogger = { debug: jest.fn() }; systemServiceProvider = mock({ configService, environment, log: jest.fn().mockReturnValue(mockLogger), }); - // Recreate the service with the updated mocks for logging tests - importService = new ImportService( + let ImportServiceWithMocks: typeof ImportService; + await jest.isolateModulesAsync(async () => { + const svc = await import("./import.service"); + ImportServiceWithMocks = svc.ImportService as typeof ImportService; + }); + + const service = new ImportServiceWithMocks( cipherService, folderService, importApiService, @@ -306,133 +360,164 @@ describe("ImportService", () => { restrictedItemTypesService, systemServiceProvider, ); - }); - afterEach(() => { - featureFlagSubject.complete(); - typeSubject.complete(); - }); + return { service, featureFlagSubject, mockLogger }; + } it("should emit metadata when type$ emits", async () => { - const testType: ImportType = "chromecsv"; + const nativeMap: Partial< + Record + > = { + chromecsv: { loaders: ["file"], instructions: "chromium" }, + }; + const { service } = await createServiceWithNativeMap(nativeMap); - const metadataPromise = firstValueFrom(importService.metadata$(typeSubject)); - typeSubject.next(testType); + const typeSubject = new Subject(); + const promise = firstValueFrom(service.metadata$(typeSubject)); + typeSubject.next("chromecsv"); + const result: ImporterMetadata = await promise; - const result = await metadataPromise; - - expect(result).toEqual({ - type: testType, - loaders: expect.any(Array), - instructions: Instructions.chromium, - }); - expect(result.type).toBe(testType); + expect(result).toEqual( + expect.objectContaining({ + type: "chromecsv", + loaders: expect.any(Array), + }), + ); + expect(result.instructions).toBeDefined(); + expect(result.type).toBe("chromecsv"); }); it("should include all loaders when chromium feature flag is enabled", async () => { - const testType: ImportType = "bravecsv"; // bravecsv supports both file and chromium loaders + // bravecsv supports both file and chromium loaders + const nativeMap: Partial< + Record + > = { + bravecsv: { loaders: ["file", "chromium"], instructions: "chromium" }, + }; + const { service, featureFlagSubject } = await createServiceWithNativeMap(nativeMap); featureFlagSubject.next(true); - const metadataPromise = firstValueFrom(importService.metadata$(typeSubject)); - typeSubject.next(testType); + const typeSubject = new Subject(); + const promise = firstValueFrom(service.metadata$(typeSubject)); + typeSubject.next("bravecsv"); + const result: ImporterMetadata = await promise; - const result = await metadataPromise; - - expect(result.loaders).toContain(Loader.chromium); - expect(result.loaders).toContain(Loader.file); + expect(result.loaders).toContain("chromium"); + expect(result.loaders).toContain("file"); }); it("should exclude chromium loader when feature flag is disabled", async () => { - const testType: ImportType = "bravecsv"; // bravecsv supports both file and chromium loaders + const nativeMap: Partial< + Record + > = { + bravecsv: { loaders: ["file", "chromium"], instructions: "chromium" }, + }; + const { service, featureFlagSubject } = await createServiceWithNativeMap(nativeMap); featureFlagSubject.next(false); - const metadataPromise = firstValueFrom(importService.metadata$(typeSubject)); - typeSubject.next(testType); + const typeSubject = new Subject(); + const promise = firstValueFrom(service.metadata$(typeSubject)); + typeSubject.next("bravecsv"); + const result: ImporterMetadata = await promise; - const result = await metadataPromise; - - expect(result.loaders).not.toContain(Loader.chromium); - expect(result.loaders).toContain(Loader.file); + expect(result.loaders).not.toContain("chromium"); + expect(result.loaders).toContain("file"); }); it("should update when type$ changes", async () => { + const nativeMap: Partial< + Record + > = { + chromecsv: { loaders: ["file"], instructions: "chromium" }, + bravecsv: { loaders: ["file", "chromium"], instructions: "chromium" }, + }; + const { service } = await createServiceWithNativeMap(nativeMap); + const emissions: ImporterMetadata[] = []; - const subscription = importService.metadata$(typeSubject).subscribe((metadata) => { - emissions.push(metadata); - }); + const typeSubject = new Subject(); + const subscription = service + .metadata$(typeSubject) + .subscribe((m: ImporterMetadata) => emissions.push(m)); typeSubject.next("chromecsv"); typeSubject.next("bravecsv"); - - // Wait for emissions - await new Promise((resolve) => setTimeout(resolve, 0)); + await new Promise((r) => setTimeout(r, 0)); expect(emissions).toHaveLength(2); expect(emissions[0].type).toBe("chromecsv"); expect(emissions[1].type).toBe("bravecsv"); - subscription.unsubscribe(); }); it("should update when feature flag changes", async () => { - const testType: ImportType = "bravecsv"; // Use bravecsv which supports chromium loader + const nativeMap: Partial< + Record + > = { + bravecsv: { loaders: ["file", "chromium"], instructions: "chromium" }, + }; + const { service, featureFlagSubject } = await createServiceWithNativeMap(nativeMap); + const emissions: ImporterMetadata[] = []; + const typeSubject = new Subject(); + const subscription = service + .metadata$(typeSubject) + .subscribe((m: ImporterMetadata) => emissions.push(m)); - const subscription = importService.metadata$(typeSubject).subscribe((metadata) => { - emissions.push(metadata); - }); - - typeSubject.next(testType); + typeSubject.next("bravecsv"); featureFlagSubject.next(true); - - // Wait for emissions - await new Promise((resolve) => setTimeout(resolve, 0)); + await new Promise((r) => setTimeout(r, 0)); expect(emissions).toHaveLength(2); - expect(emissions[0].loaders).not.toContain(Loader.chromium); - expect(emissions[1].loaders).toContain(Loader.chromium); - + expect(emissions[0].loaders).not.toContain("chromium"); + expect(emissions[1].loaders).toContain("chromium"); subscription.unsubscribe(); }); it("should update when both type$ and feature flag change", async () => { + const nativeMap: Partial< + Record + > = { + chromecsv: { loaders: ["file"], instructions: "chromium" }, + bravecsv: { loaders: ["file", "chromium"], instructions: "chromium" }, + }; + const { service, featureFlagSubject } = await createServiceWithNativeMap(nativeMap); + const emissions: ImporterMetadata[] = []; + const typeSubject = new Subject(); + const subscription = service + .metadata$(typeSubject) + .subscribe((m: ImporterMetadata) => emissions.push(m)); - const subscription = importService.metadata$(typeSubject).subscribe((metadata) => { - emissions.push(metadata); - }); - - // Initial emission typeSubject.next("chromecsv"); - - // Change both at the same time featureFlagSubject.next(true); typeSubject.next("bravecsv"); - - // Wait for emissions - await new Promise((resolve) => setTimeout(resolve, 0)); + await new Promise((r) => setTimeout(r, 0)); expect(emissions.length).toBeGreaterThanOrEqual(2); const lastEmission = emissions[emissions.length - 1]; expect(lastEmission.type).toBe("bravecsv"); - subscription.unsubscribe(); }); it("should log debug information with correct data", async () => { - const testType: ImportType = "chromecsv"; + const nativeMap: Partial< + Record + > = { + chromecsv: { loaders: ["file"], instructions: "chromium" }, + }; + const { service, mockLogger } = await createServiceWithNativeMap(nativeMap); - const metadataPromise = firstValueFrom(importService.metadata$(typeSubject)); - typeSubject.next(testType); - - await metadataPromise; + const typeSubject = new Subject(); + const promise = firstValueFrom(service.metadata$(typeSubject)); + typeSubject.next("chromecsv"); + await promise; expect(mockLogger.debug).toHaveBeenCalledWith( - { importType: testType, capabilities: expect.any(Object) }, + { importType: "chromecsv", capabilities: expect.any(Object) }, "capabilities updated", ); }); - }); */ + }); }); function createCipher(options: Partial = {}) { diff --git a/libs/importer/src/util.spec.ts b/libs/importer/src/util.spec.ts deleted file mode 100644 index 3dc80016e29..00000000000 --- a/libs/importer/src/util.spec.ts +++ /dev/null @@ -1,68 +0,0 @@ -// import { ClientType } from "@bitwarden/client-type"; - -// import { Loader } from "./metadata"; -// import { availableLoaders } from "./util"; - -// TODO Recreate metadata capability tests in Rust (source of truth), then remove this disable. -/* -describe("availableLoaders", () => { - describe("given valid import types", () => { - it("returns available loaders when client supports all loaders", () => { - const result = availableLoaders("operacsv", ClientType.Desktop); - - expect(result).toEqual([Loader.file, Loader.chromium]); - }); - - it("returns filtered loaders when client supports some loaders", () => { - const result = availableLoaders("operacsv", ClientType.Browser); - - expect(result).toEqual([Loader.file]); - }); - - it("returns single loader for import types with one loader", () => { - const result = availableLoaders("chromecsv", ClientType.Desktop); - - expect(result).toEqual([Loader.file]); - }); - - it("returns all supported loaders for multi-loader import types", () => { - const result = availableLoaders("bravecsv", ClientType.Desktop); - - expect(result).toEqual([Loader.file, Loader.chromium]); - }); - }); - - describe("given unknown import types", () => { - it("returns undefined when import type is not found in metadata", () => { - const result = availableLoaders("nonexistent" as any, ClientType.Desktop); - - expect(result).toBeUndefined(); - }); - }); - - describe("given different client types", () => { - it("returns appropriate loaders for Browser client", () => { - const result = availableLoaders("operacsv", ClientType.Browser); - - expect(result).toEqual([Loader.file]); - }); - - it("returns appropriate loaders for Web client", () => { - const result = availableLoaders("chromecsv", ClientType.Web); - - expect(result).toEqual([Loader.file]); - }); - - it("returns appropriate loaders for CLI client", () => { - const result = availableLoaders("vivaldicsv", ClientType.Cli); - - expect(result).toEqual([Loader.file]); - }); - }); -}); -*/ - -it.skip("placeholder: metadata tests moved to Rust", () => { - // TODO(rust): Port availableLoaders tests to Rust - expect(true).toBe(true); -});