Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hackathon: msal-common - Replaced sinon with jest #7341

Merged
merged 6 commits into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "Replaced sinon with jest in all the msal-common tests #7341",
"packageName": "@azure/msal-common",
"email": "[email protected]",
"dependentChangeType": "none"
}
2 changes: 0 additions & 2 deletions lib/msal-common/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@
"@types/jest": "^29.5.0",
"@types/lodash": "^4.14.182",
"@types/node": "^20.3.1",
"@types/sinon": "^7.5.0",
"eslint-config-msal": "file:../../shared-configs/eslint-config-msal",
"jest": "^29.5.0",
"lodash": "^4.17.21",
Expand All @@ -117,7 +116,6 @@
"rollup": "^3.29.5",
"rollup-msal": "file:../../shared-configs/rollup-msal",
"shx": "^0.3.2",
"sinon": "^7.5.0",
"ts-jest": "^29.1.0",
"ts-jest-resolver": "^2.0.1",
"ts-node": "^10.9.1",
Expand Down
78 changes: 37 additions & 41 deletions lib/msal-common/test/cache/CacheManager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,11 @@
import {
AuthenticationScheme,
CredentialType,
} from "../../src/utils/Constants";
import { AccountEntity } from "../../src/cache/entities/AccountEntity";
import { AccessTokenEntity } from "../../src/cache/entities/AccessTokenEntity";
import { CacheRecord } from "../../src/cache/entities/CacheRecord";
import { AccountFilter } from "../../src/cache/utils/CacheTypes";
import sinon from "sinon";
} from "../../src/utils/Constants.js";
import { AccountEntity } from "../../src/cache/entities/AccountEntity.js";
import { AccessTokenEntity } from "../../src/cache/entities/AccessTokenEntity.js";
import { CacheRecord } from "../../src/cache/entities/CacheRecord.js";
import { AccountFilter } from "../../src/cache/utils/CacheTypes.js";
import {
TEST_CONFIG,
TEST_TOKENS,
Expand All @@ -24,29 +23,29 @@ import {
TEST_TOKEN_LIFETIMES,
ID_TOKEN_ALT_CLAIMS,
GUEST_ID_TOKEN_CLAIMS,
} from "../test_kit/StringConstants";
} from "../test_kit/StringConstants.js";
import {
ClientAuthErrorCodes,
createClientAuthError,
} from "../../src/error/ClientAuthError";
import { AccountInfo } from "../../src/account/AccountInfo";
import { MockCache } from "./MockCache";
} from "../../src/error/ClientAuthError.js";
import { AccountInfo } from "../../src/account/AccountInfo.js";
import { MockCache } from "./MockCache.js";
import { buildAccountFromIdTokenClaims, buildIdToken } from "msal-test-utils";
import { mockCrypto } from "../client/ClientTestUtils";
import { TestError } from "../test_kit/TestErrors";
import { CacheManager } from "../../src/cache/CacheManager";
import { AuthorityMetadataEntity } from "../../src/cache/entities/AuthorityMetadataEntity";
import { AppMetadataEntity } from "../../src/cache/entities/AppMetadataEntity";
import { RefreshTokenEntity } from "../../src/cache/entities/RefreshTokenEntity";
import { IdTokenEntity } from "../../src/cache/entities/IdTokenEntity";
import { mockCrypto } from "../client/ClientTestUtils.js";
import { TestError } from "../test_kit/TestErrors.js";
import { CacheManager } from "../../src/cache/CacheManager.js";
import { AuthorityMetadataEntity } from "../../src/cache/entities/AuthorityMetadataEntity.js";
import { AppMetadataEntity } from "../../src/cache/entities/AppMetadataEntity.js";
import { RefreshTokenEntity } from "../../src/cache/entities/RefreshTokenEntity.js";
import { IdTokenEntity } from "../../src/cache/entities/IdTokenEntity.js";
import {
CacheHelpers,
CommonSilentFlowRequest,
PerformanceEvents,
ScopeSet,
} from "../../src";
import * as authorityMetadata from "../../src/authority/AuthorityMetadata";
import { MockPerformanceClient } from "../telemetry/PerformanceClient.spec";
} from "../../src/index.js";
import * as authorityMetadata from "../../src/authority/AuthorityMetadata.js";
import { MockPerformanceClient } from "../telemetry/PerformanceClient.spec.js";

describe("CacheManager.ts test cases", () => {
const mockCache = new MockCache(CACHE_MOCKS.MOCK_CLIENT_ID, mockCrypto, {
Expand All @@ -55,12 +54,12 @@ describe("CacheManager.ts test cases", () => {
.metadata,
knownAuthorities: [TEST_CONFIG.validAuthorityHost],
});
let authorityMetadataStub: sinon.SinonStub;
let authorityMetadataStub: jest.SpyInstance;
beforeEach(() => {
mockCache.initializeCache();
authorityMetadataStub = sinon
.stub(CacheManager.prototype, "getAuthorityMetadataByAlias")
.callsFake((host) => {
authorityMetadataStub = jest
.spyOn(CacheManager.prototype, "getAuthorityMetadataByAlias")
.mockImplementation((host) => {
const authorityMetadata: AuthorityMetadataEntity = {
aliases: [host],
preferred_cache: host,
Expand All @@ -82,7 +81,7 @@ describe("CacheManager.ts test cases", () => {

afterEach(async () => {
await mockCache.clearCache();
sinon.restore();
jest.restoreAllMocks();
});

describe("saveCacheRecord tests", () => {
Expand Down Expand Up @@ -767,7 +766,7 @@ describe("CacheManager.ts test cases", () => {
mockCache.cacheManager.getAccountsFilteredBy(successFilter);
// Both cached accounts have environments that are aliases of eachother, expect both to match
expect(Object.keys(accounts).length).toEqual(2);
sinon.restore();
jest.restoreAllMocks();

const wrongFilter: AccountFilter = { environment: "Wrong Env" };
accounts =
Expand Down Expand Up @@ -807,23 +806,23 @@ describe("CacheManager.ts test cases", () => {
});

describe("isCredentialKey", () => {
it("Returns false if key doesn't contain enough '-' deliniated sections", () => {
it("Returns false if key does not contain enough '-' deliniated sections", () => {
expect(
mockCache.cacheManager.isCredentialKey(
"clientid-idToken-homeId"
)
).toBe(false);
});

it("Returns false if key doesn't contain a valid credential type", () => {
it("Returns false if key does not contain a valid credential type", () => {
expect(
mockCache.cacheManager.isCredentialKey(
`homeAccountId-environment-credentialType-${CACHE_MOCKS.MOCK_CLIENT_ID}-realm-target-requestedClaimsHash-scheme`
)
).toBe(false);
});

it("Returns false if key doesn't contain clientId", () => {
it("Returns false if key does not contain clientId", () => {
expect(
mockCache.cacheManager.isCredentialKey(
`homeAccountId-environment-accessToken-clientId-realm-target-requestedClaimsHash-scheme`
Expand Down Expand Up @@ -937,9 +936,6 @@ describe("CacheManager.ts test cases", () => {
});

describe("environment filter", () => {
afterEach(() => {
jest.restoreAllMocks();
});
it("with configured static cloud discovery metadata", () => {
// filter by environment
expect(
Expand Down Expand Up @@ -1002,7 +998,7 @@ describe("CacheManager.ts test cases", () => {
).mockReturnValueOnce(null);
});

it("ID token matches when filter contains it's own environment", () => {
it("ID token matches when filter contains its own environment", () => {
// filter by environment
expect(
mockCache.cacheManager.credentialMatchesFilter(
Expand All @@ -1014,7 +1010,7 @@ describe("CacheManager.ts test cases", () => {
).toBe(true);
});

it("Access token matches when filter contains it's own enviroment", () => {
it("Access token matches when filter contains its own enviroment", () => {
expect(
mockCache.cacheManager.credentialMatchesFilter(
testAccessToken,
Expand All @@ -1025,7 +1021,7 @@ describe("CacheManager.ts test cases", () => {
).toBe(true);
});

it("Refresh token matches when filter contains it's own environment", () => {
it("Refresh token matches when filter contains its own environment", () => {
expect(
mockCache.cacheManager.credentialMatchesFilter(
testRefreshToken,
Expand Down Expand Up @@ -1593,7 +1589,7 @@ describe("CacheManager.ts test cases", () => {
tokenType: AuthenticationScheme.POP,
};

const removeTokenBindingKeySpy = sinon.spy(
const removeTokenBindingKeySpy = jest.spyOn(
mockCrypto,
"removeTokenBindingKey"
);
Expand All @@ -1603,7 +1599,7 @@ describe("CacheManager.ts test cases", () => {
);
const atKey = CacheHelpers.generateCredentialKey(atWithAuthScheme);
expect(mockCache.cacheManager.getAccount(atKey)).toBeNull();
expect(removeTokenBindingKeySpy.getCall(0).args[0]).toEqual(
expect(removeTokenBindingKeySpy.mock.calls[0][0]).toEqual(
atWithAuthScheme.keyId
);
});
Expand All @@ -1624,7 +1620,7 @@ describe("CacheManager.ts test cases", () => {
tokenType: AuthenticationScheme.SSH,
};

const removeTokenBindingKeySpy = sinon.spy(
const removeTokenBindingKeySpy = jest.spyOn(
mockCrypto,
"removeTokenBindingKey"
);
Expand All @@ -1634,10 +1630,10 @@ describe("CacheManager.ts test cases", () => {
);
const atKey = CacheHelpers.generateCredentialKey(atWithAuthScheme);
expect(mockCache.cacheManager.getAccount(atKey)).toBeNull();
expect(removeTokenBindingKeySpy.callCount).toEqual(0);
expect(removeTokenBindingKeySpy).toHaveBeenCalledTimes(0);
});

it("throws bindingKeyNotRemoved error when key isn't deleted from storage", async () => {
it("throws bindingKeyNotRemoved error when key is not deleted from storage", async () => {
const atWithAuthScheme = {
environment: "login.microsoftonline.com",
credentialType: CredentialType.ACCESS_TOKEN_WITH_AUTH_SCHEME,
Expand Down Expand Up @@ -2207,7 +2203,7 @@ describe("CacheManager.ts test cases", () => {
});

it("getRefreshToken with environment aliases", () => {
authorityMetadataStub.callsFake((host) => {
authorityMetadataStub.mockImplementation((host) => {
const authorityMetadata: AuthorityMetadataEntity = {
aliases: ["login.microsoftonline.com", "login.windows.net"],
preferred_cache: host,
Expand Down
68 changes: 36 additions & 32 deletions lib/msal-common/test/cache/entities/AccountEntity.spec.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { AccountEntity } from "../../../src/cache/entities/AccountEntity";
import { mockAccountEntity, mockIdTokenEntity } from "./cacheConstants";
import * as AuthToken from "../../../src/account/AuthToken";
import { CacheAccountType, Constants } from "../../../src/utils/Constants";
import { AccountEntity } from "../../../src/cache/entities/AccountEntity.js";
import { mockAccountEntity, mockIdTokenEntity } from "./cacheConstants.js";
import * as AuthToken from "../../../src/account/AuthToken.js";
import { CacheAccountType, Constants } from "../../../src/utils/Constants.js";
import {
NetworkRequestOptions,
INetworkModule,
} from "../../../src/network/INetworkModule";
import { ICrypto } from "../../../src/crypto/ICrypto";
} from "../../../src/network/INetworkModule.js";
import { ICrypto } from "../../../src/crypto/ICrypto.js";
import {
RANDOM_TEST_GUID,
TEST_DATA_CLIENT_INFO,
Expand All @@ -18,16 +18,18 @@ import {
ID_TOKEN_CLAIMS,
GUEST_ID_TOKEN_CLAIMS,
TEST_CONFIG,
} from "../../test_kit/StringConstants";
import sinon from "sinon";
import { MockStorageClass, mockCrypto } from "../../client/ClientTestUtils";
import { AccountInfo, TenantProfile } from "../../../src/account/AccountInfo";
import { AuthorityOptions } from "../../../src/authority/AuthorityOptions";
import { ProtocolMode } from "../../../src/authority/ProtocolMode";
import { LogLevel, Logger } from "../../../src/logger/Logger";
import { Authority } from "../../../src/authority/Authority";
import { AuthorityType } from "../../../src/authority/AuthorityType";
import { TokenClaims } from "../../../src";
} from "../../test_kit/StringConstants.js";
import { MockStorageClass, mockCrypto } from "../../client/ClientTestUtils.js";
import {
AccountInfo,
TenantProfile,
} from "../../../src/account/AccountInfo.js";
import { AuthorityOptions } from "../../../src/authority/AuthorityOptions.js";
import { ProtocolMode } from "../../../src/authority/ProtocolMode.js";
import { LogLevel, Logger } from "../../../src/logger/Logger.js";
import { Authority } from "../../../src/authority/Authority.js";
import { AuthorityType } from "../../../src/authority/AuthorityType.js";
import { TokenClaims } from "../../../src/index.js";
import { buildAccountFromIdTokenClaims } from "msal-test-utils";

const cryptoInterface: ICrypto = {
Expand Down Expand Up @@ -131,13 +133,13 @@ const authority = new Authority(

describe("AccountEntity.ts Unit Tests", () => {
beforeEach(() => {
sinon
.stub(Authority.prototype, "getPreferredCache")
.returns("login.windows.net");
jest.spyOn(Authority.prototype, "getPreferredCache").mockReturnValue(
"login.windows.net"
);
});

afterEach(() => {
sinon.restore();
jest.restoreAllMocks();
});

it("Verify an AccountEntity", () => {
Expand Down Expand Up @@ -344,7 +346,9 @@ describe("AccountEntity.ts Unit Tests", () => {
nonce: "123523",
upn: "testupn",
};
sinon.stub(AuthToken, "extractTokenClaims").returns(idTokenClaims);
jest.spyOn(AuthToken, "extractTokenClaims").mockReturnValue(
idTokenClaims
);

const homeAccountId =
"AAAAAAAAAAAAAAAAAAAAAIkzqFVrSaSaFHy782bbtaQ".toLowerCase();
Expand Down Expand Up @@ -396,7 +400,7 @@ describe("AccountEntity.ts Unit Tests", () => {
expect(accountInfo.tenantProfiles).toMatchObject(tenantProfiles);
});

it("getAccountInfo creates a new tenantProfiles map if AccountEntity doesn't have a tenantProfiles array", () => {
it("getAccountInfo creates a new tenantProfiles map if AccountEntity does not have a tenantProfiles array", () => {
const accountEntity = buildAccountFromIdTokenClaims(ID_TOKEN_CLAIMS);
accountEntity.tenantProfiles = undefined;

Expand All @@ -408,7 +412,7 @@ describe("AccountEntity.ts Unit Tests", () => {
);
});

it("isSingleTenant returns true if AccountEntity doesn't have a tenantProfiles array", () => {
it("isSingleTenant returns true if AccountEntity does not have a tenantProfiles array", () => {
const accountEntity = buildAccountFromIdTokenClaims(ID_TOKEN_CLAIMS);
accountEntity.tenantProfiles = undefined;

Expand Down Expand Up @@ -638,13 +642,9 @@ describe("AccountEntity.ts Unit Tests", () => {

describe("AccountEntity.ts Unit Tests for ADFS", () => {
beforeEach(() => {
sinon
.stub(Authority.prototype, "getPreferredCache")
.returns("myadfs.com");
});

afterEach(() => {
sinon.restore();
jest.spyOn(Authority.prototype, "getPreferredCache").mockReturnValue(
"myadfs.com"
);
});

it("creates a generic ADFS account", () => {
Expand Down Expand Up @@ -674,7 +674,9 @@ describe("AccountEntity.ts Unit Tests for ADFS", () => {
nonce: "123523",
upn: "testupn",
};
sinon.stub(AuthToken, "extractTokenClaims").returns(idTokenClaims);
jest.spyOn(AuthToken, "extractTokenClaims").mockReturnValue(
idTokenClaims
);

const homeAccountId =
"AAAAAAAAAAAAAAAAAAAAAIkzqFVrSaSaFHy782bbtaQ".toLowerCase();
Expand Down Expand Up @@ -727,7 +729,9 @@ describe("AccountEntity.ts Unit Tests for ADFS", () => {
nonce: "123523",
upn: "testupn",
};
sinon.stub(AuthToken, "extractTokenClaims").returns(idTokenClaims);
jest.spyOn(AuthToken, "extractTokenClaims").mockReturnValue(
idTokenClaims
);

const homeAccountId =
"AAAAAAAAAAAAAAAAAAAAAIkzqFVrSaSaFHy782bbtaQ".toLowerCase();
Expand Down
Loading
Loading