Skip to content

Commit

Permalink
[ADR-0006][AC-319] Migrate all tests to use jest mock instead of subs…
Browse files Browse the repository at this point in the history
…titute (#6520)

Standardize on using jest mock instead of having two mocking frameworks which can be confusing.
  • Loading branch information
Hinton authored Oct 17, 2023
1 parent 5cacd79 commit ffb67be
Show file tree
Hide file tree
Showing 17 changed files with 199 additions and 209 deletions.
5 changes: 1 addition & 4 deletions .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,7 @@
]
}
],
"no-restricted-imports": [
"error",
{ "patterns": ["src/**/*"], "paths": ["@fluffy-spoon/substitute"] }
]
"no-restricted-imports": ["error", { "patterns": ["src/**/*"] }]
}
},
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,18 @@
// eslint-disable-next-line no-restricted-imports
import { Arg, Substitute, SubstituteOf } from "@fluffy-spoon/substitute";
import { mock, MockProxy } from "jest-mock-extended";

import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service";
import { Utils } from "@bitwarden/common/platform/misc/utils";
import { EncString } from "@bitwarden/common/platform/models/domain/enc-string";
import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key";
import { EncryptServiceImplementation } from "@bitwarden/common/platform/services/cryptography/encrypt.service.implementation";

import BrowserLocalStorageService from "./browser-local-storage.service";
import BrowserMemoryStorageService from "./browser-memory-storage.service";
import { KeyGenerationService } from "./key-generation.service";
import { LocalBackedSessionStorageService } from "./local-backed-session-storage.service";

describe("Browser Session Storage Service", () => {
let encryptService: SubstituteOf<EncryptServiceImplementation>;
let keyGenerationService: SubstituteOf<KeyGenerationService>;
let encryptService: MockProxy<EncryptService>;
let keyGenerationService: MockProxy<KeyGenerationService>;

let cache: Map<string, any>;
const testObj = { a: 1, b: 2 };
Expand All @@ -28,8 +27,8 @@ describe("Browser Session Storage Service", () => {
let sut: LocalBackedSessionStorageService;

beforeEach(() => {
encryptService = Substitute.for();
keyGenerationService = Substitute.for();
encryptService = mock<EncryptService>();
keyGenerationService = mock<KeyGenerationService>();

sut = new LocalBackedSessionStorageService(encryptService, keyGenerationService);

Expand Down Expand Up @@ -138,6 +137,8 @@ describe("Browser Session Storage Service", () => {
jest.spyOn(sessionStorage, "get").mockResolvedValue(null);
jest.spyOn(localStorage, "save").mockResolvedValue();
jest.spyOn(sessionStorage, "save").mockResolvedValue();

encryptService.encrypt.mockResolvedValue(mockEnc("{}"));
});

it("should remove key from cache if value is null", async () => {
Expand Down Expand Up @@ -205,15 +206,15 @@ describe("Browser Session Storage Service", () => {
describe("new key creation", () => {
beforeEach(() => {
jest.spyOn(sessionStorage, "get").mockResolvedValue(null);
keyGenerationService.makeEphemeralKey().resolves(key);
keyGenerationService.makeEphemeralKey.mockResolvedValue(key);
jest.spyOn(sut, "setSessionEncKey").mockResolvedValue();
});

it("should create a symmetric crypto key", async () => {
const result = await sut.getSessionEncKey();

expect(result).toStrictEqual(key);
keyGenerationService.received(1).makeEphemeralKey();
expect(keyGenerationService.makeEphemeralKey).toBeCalledTimes(1);
});

it("should store a symmetric crypto key if it makes one", async () => {
Expand Down Expand Up @@ -244,19 +245,23 @@ describe("Browser Session Storage Service", () => {
});

it("should decrypt returned sessions", async () => {
encryptService.decryptToUtf8(encSession, key).resolves(decryptedSession);
encryptService.decryptToUtf8
.calledWith(expect.anything(), key)
.mockResolvedValue(decryptedSession);
await sut.getLocalSession(key);
encryptService.received(1).decryptToUtf8(encSession, key);
expect(encryptService.decryptToUtf8).toHaveBeenNthCalledWith(1, encSession, key);
});

it("should parse session", async () => {
encryptService.decryptToUtf8(encSession, key).resolves(decryptedSession);
encryptService.decryptToUtf8
.calledWith(expect.anything(), key)
.mockResolvedValue(decryptedSession);
const result = await sut.getLocalSession(key);
expect(result).toEqual(session);
});

it("should remove state if decryption fails", async () => {
encryptService.decryptToUtf8(Arg.any(), Arg.any()).resolves(null);
encryptService.decryptToUtf8.mockResolvedValue(null);
const setSessionEncKeySpy = jest.spyOn(sut, "setSessionEncKey").mockResolvedValue();
const removeLocalSessionSpy = jest.spyOn(localStorage, "remove").mockResolvedValue();

Expand All @@ -274,23 +279,23 @@ describe("Browser Session Storage Service", () => {
const testJSON = JSON.stringify(testSession);

it("should encrypt a stringified session", async () => {
encryptService.encrypt(Arg.any(), Arg.any()).mimicks(mockEnc);
encryptService.encrypt.mockImplementation(mockEnc);
jest.spyOn(localStorage, "save").mockResolvedValue();
await sut.setLocalSession(testSession, key);

encryptService.received(1).encrypt(testJSON, key);
expect(encryptService.encrypt).toHaveBeenNthCalledWith(1, testJSON, key);
});

it("should remove local session if null", async () => {
encryptService.encrypt(Arg.any(), Arg.any()).resolves(null);
encryptService.encrypt.mockResolvedValue(null);
const spy = jest.spyOn(localStorage, "remove").mockResolvedValue();
await sut.setLocalSession(null, key);

expect(spy).toHaveBeenCalledWith("session");
});

it("should save encrypted string", async () => {
encryptService.encrypt(Arg.any(), Arg.any()).mimicks(mockEnc);
encryptService.encrypt.mockImplementation(mockEnc);
const spy = jest.spyOn(localStorage, "save").mockResolvedValue();
await sut.setLocalSession(testSession, key);

Expand Down
14 changes: 6 additions & 8 deletions apps/desktop/src/app/tools/generator.component.spec.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import { NO_ERRORS_SCHEMA } from "@angular/core";
import { ComponentFixture, TestBed } from "@angular/core/testing";
import { ActivatedRoute } from "@angular/router";
// eslint-disable-next-line no-restricted-imports
import { Substitute } from "@fluffy-spoon/substitute";
import { mock, MockProxy } from "jest-mock-extended";

import { I18nPipe } from "@bitwarden/angular/platform/pipes/i18n.pipe";
Expand All @@ -28,31 +26,31 @@ describe("GeneratorComponent", () => {
providers: [
{
provide: PasswordGenerationServiceAbstraction,
useClass: Substitute.for<PasswordGenerationServiceAbstraction>(),
useValue: mock<PasswordGenerationServiceAbstraction>(),
},
{
provide: UsernameGenerationServiceAbstraction,
useClass: Substitute.for<UsernameGenerationServiceAbstraction>(),
useValue: mock<UsernameGenerationServiceAbstraction>(),
},
{
provide: StateService,
useClass: Substitute.for<StateService>(),
useValue: mock<StateService>(),
},
{
provide: PlatformUtilsService,
useValue: platformUtilsServiceMock,
},
{
provide: I18nService,
useClass: Substitute.for<I18nService>(),
useValue: mock<I18nService>(),
},
{
provide: ActivatedRoute,
useClass: Substitute.for<ActivatedRoute>(),
useValue: mock<ActivatedRoute>(),
},
{
provide: LogService,
useClass: Substitute.for<LogService>(),
useValue: mock<LogService>(),
},
],
schemas: [NO_ERRORS_SCHEMA],
Expand Down
3 changes: 1 addition & 2 deletions apps/web/.eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@
"@bitwarden/web-vault/*",
"src/**/*",
"bitwarden_license"
],
"paths": ["@fluffy-spoon/substitute"]
]
}
]
}
Expand Down
11 changes: 5 additions & 6 deletions libs/common/spec/utils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// eslint-disable-next-line no-restricted-imports
import { Substitute, Arg } from "@fluffy-spoon/substitute";
import { mock, MockProxy } from "jest-mock-extended";

import { EncString } from "@bitwarden/common/platform/models/domain/enc-string";

Expand All @@ -22,11 +21,11 @@ export function BuildTestObject<T, K extends keyof T = keyof T>(
return Object.assign(constructor === null ? {} : new constructor(), def) as T;
}

export function mockEnc(s: string): EncString {
const mock = Substitute.for<EncString>();
mock.decrypt(Arg.any(), Arg.any()).resolves(s);
export function mockEnc(s: string): MockProxy<EncString> {
const mocked = mock<EncString>();
mocked.decrypt.mockResolvedValue(s);

return mock;
return mocked;
}

export function makeStaticByteArray(length: number, start = 0) {
Expand Down
18 changes: 11 additions & 7 deletions libs/common/src/platform/models/domain/enc-string.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// eslint-disable-next-line no-restricted-imports
import { Substitute, Arg } from "@fluffy-spoon/substitute";
import { mock, MockProxy } from "jest-mock-extended";

import { makeStaticByteArray } from "../../../../spec";
import { EncryptionType } from "../../../enums";
import { EncryptService } from "../../../platform/abstractions/encrypt.service";
import {
Expand Down Expand Up @@ -64,11 +63,16 @@ describe("EncString", () => {
describe("decrypt", () => {
const encString = new EncString(EncryptionType.Rsa2048_OaepSha256_B64, "data");

const cryptoService = Substitute.for<CryptoService>();
cryptoService.getOrgKey(null).resolves(null);
const cryptoService = mock<CryptoService>();
cryptoService.hasUserKey.mockResolvedValue(true);
cryptoService.getUserKeyWithLegacySupport.mockResolvedValue(
new SymmetricCryptoKey(makeStaticByteArray(32)) as UserKey
);

const encryptService = Substitute.for<EncryptService>();
encryptService.decryptToUtf8(encString, Arg.any()).resolves("decrypted");
const encryptService = mock<EncryptService>();
encryptService.decryptToUtf8
.calledWith(encString, expect.anything())
.mockResolvedValue("decrypted");

beforeEach(() => {
(window as any).bitwardenContainerService = new ContainerService(
Expand All @@ -85,7 +89,7 @@ describe("EncString", () => {

it("result should be cached", async () => {
const decrypted = await encString.decrypt(null);
encryptService.received(1).decryptToUtf8(Arg.any(), Arg.any());
expect(encryptService.decryptToUtf8).toBeCalledTimes(1);

expect(decrypted).toBe("decrypted");
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// eslint-disable-next-line no-restricted-imports
import { Substitute } from "@fluffy-spoon/substitute";
import { mock } from "jest-mock-extended";

import { Utils } from "../../platform/misc/utils";
import { PlatformUtilsService } from "../abstractions/platform-utils.service";
Expand Down Expand Up @@ -582,8 +581,8 @@ function testRsaGenerateKeyPair(length: 1024 | 2048 | 4096) {
}

function getWebCryptoFunctionService() {
const platformUtilsMock = Substitute.for<PlatformUtilsService>();
platformUtilsMock.isEdge().mimicks(() => navigator.userAgent.indexOf(" Edg/") !== -1);
const platformUtilsMock = mock<PlatformUtilsService>();
platformUtilsMock.isEdge.mockImplementation(() => navigator.userAgent.indexOf(" Edg/") !== -1);

return new WebCryptoFunctionService(window);
}
Expand Down
43 changes: 21 additions & 22 deletions libs/common/src/services/policy.service.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// eslint-disable-next-line no-restricted-imports
import { Arg, Substitute, SubstituteOf } from "@fluffy-spoon/substitute";
import { mock, MockProxy } from "jest-mock-extended";
import { BehaviorSubject, firstValueFrom } from "rxjs";

import { OrganizationService } from "../admin-console/abstractions/organization/organization.service.abstraction";
Expand All @@ -22,19 +21,19 @@ import { StateService } from "../platform/services/state.service";
describe("PolicyService", () => {
let policyService: PolicyService;

let cryptoService: SubstituteOf<CryptoService>;
let stateService: SubstituteOf<StateService>;
let organizationService: SubstituteOf<OrganizationService>;
let encryptService: SubstituteOf<EncryptService>;
let cryptoService: MockProxy<CryptoService>;
let stateService: MockProxy<StateService>;
let organizationService: MockProxy<OrganizationService>;
let encryptService: MockProxy<EncryptService>;
let activeAccount: BehaviorSubject<string>;
let activeAccountUnlocked: BehaviorSubject<boolean>;

beforeEach(() => {
stateService = Substitute.for();
organizationService = Substitute.for();
organizationService
.getAll("user")
.resolves([
stateService = mock<StateService>();
organizationService = mock<OrganizationService>();
organizationService.getAll
.calledWith("user")
.mockResolvedValue([
new Organization(
organizationData(
"test-organization",
Expand All @@ -45,24 +44,24 @@ describe("PolicyService", () => {
)
),
]);
organizationService.getAll(undefined).resolves([]);
organizationService.getAll(null).resolves([]);
organizationService.getAll.calledWith(undefined).mockResolvedValue([]);
organizationService.getAll.calledWith(null).mockResolvedValue([]);
activeAccount = new BehaviorSubject("123");
activeAccountUnlocked = new BehaviorSubject(true);
stateService.getDecryptedPolicies({ userId: "user" }).resolves(null);
stateService.getEncryptedPolicies({ userId: "user" }).resolves({
stateService.getDecryptedPolicies.calledWith({ userId: "user" }).mockResolvedValue(null);
stateService.getEncryptedPolicies.calledWith({ userId: "user" }).mockResolvedValue({
"1": policyData("1", "test-organization", PolicyType.MaximumVaultTimeout, true, {
minutes: 14,
}),
});
stateService.getEncryptedPolicies().resolves({
stateService.getEncryptedPolicies.mockResolvedValue({
"1": policyData("1", "test-organization", PolicyType.MaximumVaultTimeout, true, {
minutes: 14,
}),
});
stateService.activeAccount$.returns(activeAccount);
stateService.activeAccountUnlocked$.returns(activeAccountUnlocked);
stateService.getUserId().resolves("user");
stateService.activeAccount$ = activeAccount;
stateService.activeAccountUnlocked$ = activeAccountUnlocked;
stateService.getUserId.mockResolvedValue("user");
(window as any).bitwardenContainerService = new ContainerService(cryptoService, encryptService);

policyService = new PolicyService(stateService, organizationService);
Expand Down Expand Up @@ -120,23 +119,23 @@ describe("PolicyService", () => {
it("null userId", async () => {
await policyService.clear();

stateService.received(1).setEncryptedPolicies(Arg.any(), Arg.any());
expect(stateService.setEncryptedPolicies).toBeCalledTimes(1);

expect((await firstValueFrom(policyService.policies$)).length).toBe(0);
});

it("matching userId", async () => {
await policyService.clear("user");

stateService.received(1).setEncryptedPolicies(Arg.any(), Arg.any());
expect(stateService.setEncryptedPolicies).toBeCalledTimes(1);

expect((await firstValueFrom(policyService.policies$)).length).toBe(0);
});

it("mismatching userId", async () => {
await policyService.clear("12");

stateService.received(1).setEncryptedPolicies(Arg.any(), Arg.any());
expect(stateService.setEncryptedPolicies).toBeCalledTimes(1);

expect((await firstValueFrom(policyService.policies$)).length).toBe(1);
});
Expand Down
Loading

0 comments on commit ffb67be

Please sign in to comment.