Skip to content

Commit

Permalink
test fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@kuhe
kuhe committed May 24, 2024
1 parent da6e574 commit 9d8a589
Show file tree
Hide file tree
Showing 7 changed files with 137 additions and 87 deletions.
29 changes: 17 additions & 12 deletions packages/node-config-provider/src/fromEnv.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,30 @@ import { fromEnv, GetterFromEnv } from "./fromEnv";

describe("fromEnv", () => {
describe("with env var getter", () => {
const envVarName = "ENV_VAR_NAME";
const ENV_VAR_NAME = "ENV_VAR_NAME";

// Using Record<string, string | undefined> instead of NodeJS.ProcessEnv, in order to not get type errors in non node environments
const envVarGetter: GetterFromEnv<string> = (env: Record<string, string | undefined>) => env[envVarName]!;
const envVarValue = process.env[envVarName];
const envVarGetter: GetterFromEnv<string> = (env: Record<string, string | undefined>) => env[ENV_VAR_NAME]!;
const envVarValue = process.env[ENV_VAR_NAME];
const mockEnvVarValue = "mockEnvVarValue";

const getCredentialsProviderError = (getter: GetterFromEnv<string>) =>
new CredentialsProviderError(`Cannot load config from environment variables with getter: ${getter}`);

beforeEach(() => {
delete process.env[envVarName];
delete process.env[ENV_VAR_NAME];
});

afterAll(() => {
process.env[envVarName] = envVarValue;
process.env[ENV_VAR_NAME] = envVarValue;
});

describe("CredentialsProviderError", () => {
it("is behaving as expected cross-package in jest", () => {
expect(new CredentialsProviderError("msg", {}).message).toEqual("msg");
expect(new CredentialsProviderError("msg", {}).name).toEqual("CredentialsProviderError");
});
});

it(`returns string value in '${envVarName}' env var when set`, () => {
process.env[envVarName] = mockEnvVarValue;
it(`returns string value in '${ENV_VAR_NAME}' env var when set`, () => {
process.env[ENV_VAR_NAME] = mockEnvVarValue;
return expect(fromEnv(envVarGetter)()).resolves.toBe(mockEnvVarValue);
});

Expand All @@ -35,9 +39,10 @@ describe("fromEnv", () => {
return expect(fromEnv(getter)()).resolves.toEqual(value);
});

it(`throws when '${envVarName}' env var is not set`, () => {
it(`throws when '${ENV_VAR_NAME}' env var is not set`, async () => {
expect.assertions(1);
return expect(fromEnv(envVarGetter)()).rejects.toMatchObject(getCredentialsProviderError(envVarGetter));
const error = await fromEnv(envVarGetter)().catch((_) => _);
return expect(error).toEqual(new CredentialsProviderError(`Not found in ENV: ENV_VAR_NAME`, {}));
});

it("throws when the getter function throws", () => {
Expand Down
48 changes: 23 additions & 25 deletions packages/node-config-provider/src/fromSharedConfigFiles.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,11 @@ jest.mock("@smithy/shared-ini-file-loader", () => ({
}));

describe("fromSharedConfigFiles", () => {
const configKey = "config_key";
const configGetter: GetterFromConfig<string> = (profile: Profile) => profile[configKey];
const CONFIG_KEY = "config_key";
const configGetter: GetterFromConfig<string> = (profile: Profile) => profile[CONFIG_KEY];

const getCredentialsProviderError = (profile: string, getter: GetterFromConfig<string>) =>
new CredentialsProviderError(
`Cannot load config for profile ${profile} in SDK configuration files with getter: ${getter}`
);
const getCredentialsProviderError = (profile: string) =>
new CredentialsProviderError(`Not found in config files w/ profile [${profile}]: CONFIG_KEY`, {});

describe("loadedConfig", () => {
const mockConfigAnswer = "mockConfigAnswer";
Expand All @@ -36,33 +34,33 @@ describe("fromSharedConfigFiles", () => {
{
message: "returns configValue from default profile",
iniDataInConfig: {
default: { [configKey]: mockConfigAnswer },
default: { [CONFIG_KEY]: mockConfigAnswer },
},
iniDataInCredentials: {
default: { [configKey]: mockCredentialsNotAnswer },
default: { [CONFIG_KEY]: mockCredentialsNotAnswer },
},
configValueToVerify: mockConfigAnswer,
},
{
message: "returns configValue from designated profile",
iniDataInConfig: {
default: { [configKey]: mockConfigNotAnswer },
foo: { [configKey]: mockConfigAnswer },
default: { [CONFIG_KEY]: mockConfigNotAnswer },
foo: { [CONFIG_KEY]: mockConfigAnswer },
},
iniDataInCredentials: {
foo: { [configKey]: mockCredentialsNotAnswer },
foo: { [CONFIG_KEY]: mockCredentialsNotAnswer },
},
profile: "foo",
configValueToVerify: mockConfigAnswer,
},
{
message: "returns configValue from credentials file if preferred",
iniDataInConfig: {
default: { [configKey]: mockConfigNotAnswer },
foo: { [configKey]: mockConfigNotAnswer },
default: { [CONFIG_KEY]: mockConfigNotAnswer },
foo: { [CONFIG_KEY]: mockConfigNotAnswer },
},
iniDataInCredentials: {
foo: { [configKey]: mockCredentialsAnswer },
foo: { [CONFIG_KEY]: mockCredentialsAnswer },
},
profile: "foo",
preferredFile: "credentials",
Expand All @@ -71,7 +69,7 @@ describe("fromSharedConfigFiles", () => {
{
message: "returns configValue from config file if preferred credentials file doesn't contain config",
iniDataInConfig: {
foo: { [configKey]: mockConfigAnswer },
foo: { [CONFIG_KEY]: mockConfigAnswer },
},
iniDataInCredentials: {},
configValueToVerify: mockConfigAnswer,
Expand All @@ -82,7 +80,7 @@ describe("fromSharedConfigFiles", () => {
message: "returns configValue from credential file if preferred config file doesn't contain config",
iniDataInConfig: {},
iniDataInCredentials: {
foo: { [configKey]: mockCredentialsAnswer },
foo: { [CONFIG_KEY]: mockCredentialsAnswer },
},
configValueToVerify: mockCredentialsAnswer,
profile: "foo",
Expand All @@ -93,14 +91,14 @@ describe("fromSharedConfigFiles", () => {
{
message: "rejects if default profile is not present and profile value is not passed",
iniDataInConfig: {
foo: { [configKey]: mockConfigNotAnswer },
foo: { [CONFIG_KEY]: mockConfigNotAnswer },
},
iniDataInCredentials: {},
},
{
message: "rejects if designated profile is not present",
iniDataInConfig: {
default: { [configKey]: mockConfigNotAnswer },
default: { [CONFIG_KEY]: mockConfigNotAnswer },
},
iniDataInCredentials: {},
profile: "foo",
Expand Down Expand Up @@ -129,8 +127,8 @@ describe("fromSharedConfigFiles", () => {
credentialsFile: iniDataInCredentials,
});
(getProfileName as jest.Mock).mockReturnValueOnce(profile ?? "default");
return expect(fromSharedConfigFiles(configGetter, { profile, preferredFile })()).rejects.toMatchObject(
getCredentialsProviderError(profile ?? "default", configGetter)
return expect(fromSharedConfigFiles(configGetter, { profile, preferredFile })()).rejects.toEqual(
getCredentialsProviderError(profile ?? "default")
);
});
});
Expand All @@ -144,18 +142,18 @@ describe("fromSharedConfigFiles", () => {
configFile: {},
credentialsFile: {},
});
return expect(fromSharedConfigFiles(failGetter)()).rejects.toMatchObject(new CredentialsProviderError(message));
return expect(fromSharedConfigFiles(failGetter)()).rejects.toEqual(new CredentialsProviderError(message));
});
});

describe("profile", () => {
const loadedConfigData = {
configFile: {
default: { [configKey]: "configFileDefault" },
foo: { [configKey]: "configFileFoo" },
default: { [CONFIG_KEY]: "configFileDefault" },
foo: { [CONFIG_KEY]: "configFileFoo" },
},
credentialsFile: {
default: { [configKey]: "credentialsFileDefault" },
default: { [CONFIG_KEY]: "credentialsFileDefault" },
},
};

Expand All @@ -166,7 +164,7 @@ describe("fromSharedConfigFiles", () => {
it.each(["foo", "default"])("returns config value from %s profile", (profile) => {
(getProfileName as jest.Mock).mockReturnValueOnce(profile);
return expect(fromSharedConfigFiles(configGetter)()).resolves.toBe(
loadedConfigData.configFile[profile][configKey]
loadedConfigData.configFile[profile][CONFIG_KEY]
);
});
});
Expand Down
14 changes: 9 additions & 5 deletions packages/node-config-provider/src/getSelectorName.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,13 @@
* @returns constant value used within the function.
*/
export function getSelectorName(functionString: string): string {
const constants = new Set(Array.from(functionString.match(/([A-Z_]){3,}/g) ?? []));
constants.delete("CONFIG");
constants.delete("CONFIG_PREFIX_SEPARATOR");
constants.delete("ENV");
return [...constants].join(", ");
try {
const constants = new Set(Array.from(functionString.match(/([A-Z_]){3,}/g) ?? []));
constants.delete("CONFIG");
constants.delete("CONFIG_PREFIX_SEPARATOR");
constants.delete("ENV");
return [...constants].join(", ");
} catch (e) {
return functionString;
}
}
28 changes: 24 additions & 4 deletions packages/property-provider/src/CredentialsProviderError.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,31 @@ import { CredentialsProviderError } from "./CredentialsProviderError";
import { ProviderError } from "./ProviderError";

describe(CredentialsProviderError.name, () => {
it("should be named as CredentialsProviderError", () => {
it("should be named CredentialsProviderError", () => {
expect(new CredentialsProviderError("PANIC").name).toBe("CredentialsProviderError");
});

it("should use logger.trace if provided", () => {
it("should have a non-enumerable message like the base Error class", () => {
expect(new CredentialsProviderError("PANIC", {}).message).toBe("PANIC");

expect(
{
...new CredentialsProviderError("PANIC", {}),
}.message
).toBe(undefined);
});

it("should have an enumerable tryNextLink and logger like the base Error class", () => {
expect(new CredentialsProviderError("PANIC", {}).tryNextLink).toBe(true);

expect(
{
...new CredentialsProviderError("PANIC", { tryNextLink: false }),
}.tryNextLink
).toBe(false);
});

it("should use logger.debug if provided", () => {
const logger = {
info: jest.fn(),
warn: jest.fn(),
Expand All @@ -16,8 +36,8 @@ describe(CredentialsProviderError.name, () => {
};
new CredentialsProviderError("PANIC", { logger });

expect(logger.debug).not.toHaveBeenCalled();
expect(logger.trace).toHaveBeenCalled();
expect(logger.debug).toHaveBeenCalled();
expect(logger.trace).not.toHaveBeenCalled();
});

describe.each([Error, ProviderError, CredentialsProviderError])("should be instanceof %p", (classConstructor) => {
Expand Down
33 changes: 6 additions & 27 deletions packages/property-provider/src/CredentialsProviderError.ts
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,6 @@
import { Logger } from "@smithy/types";

Check failure on line 1 in packages/property-provider/src/CredentialsProviderError.ts

View workflow job for this annotation

GitHub Actions / TypeScript Lint 

'Logger' is defined but never used

import { ProviderError } from "./ProviderError";

/**
* @public
*/
export type CredentialsProviderErrorOptionsType = {
tryNextLink?: boolean | undefined;
logger?: Logger;
};
import { ProviderError, ProviderErrorOptionsType } from "./ProviderError";

/**
* @public
Expand All @@ -22,8 +14,7 @@ export type CredentialsProviderErrorOptionsType = {
* ensures the chain will stop if an entirely unexpected error is encountered.
*/
export class CredentialsProviderError extends ProviderError {
public name = "CredentialsProviderError";
public readonly tryNextLink: boolean = true;
name = "CredentialsProviderError";

/**
* @deprecated constructor should be given a logger.
Expand All @@ -32,25 +23,13 @@ export class CredentialsProviderError extends ProviderError {
/**
* @deprecated constructor should be given a logger.
*/
public constructor(message: string, tryNextLink?: boolean | undefined);
public constructor(message: string, tryNextLink: boolean | undefined);
/**
* This signature is preferred for logging capability.
*/
public constructor(message: string, options: CredentialsProviderErrorOptionsType);
public constructor(message: string, options: boolean | CredentialsProviderErrorOptionsType = true) {
let logger: Logger | undefined;
let tryNextLink: boolean = true;

if (typeof options === "boolean") {
logger = undefined;
tryNextLink = options;
} else if (options != null && typeof options === "object") {
logger = options.logger;
tryNextLink = options.tryNextLink ?? true;
}
super(message, tryNextLink);

public constructor(message: string, options: ProviderErrorOptionsType);
public constructor(message: string, options: boolean | ProviderErrorOptionsType = true) {
super(message, options as ProviderErrorOptionsType);
Object.setPrototypeOf(this, CredentialsProviderError.prototype);
logger?.trace?.(`${new Date().toISOString()} ${tryNextLink ? "->" : "(!)"} ${message}`);
}
}
50 changes: 43 additions & 7 deletions packages/property-provider/src/ProviderError.ts
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
import { Logger } from "@smithy/types";

/**
* @public
*/
export type ProviderErrorOptionsType = {
tryNextLink?: boolean | undefined;
logger?: Logger;
};

/**
* @public
*
Expand All @@ -11,15 +21,41 @@
*/
export class ProviderError extends Error {
name = "ProviderError";
constructor(
message: string,
public readonly tryNextLink: boolean = true
) {
public readonly tryNextLink: boolean;

/**
* @deprecated constructor should be given a logger.
*/
public constructor(message: string);
/**
* @deprecated constructor should be given a logger.
*/
public constructor(message: string, tryNextLink: boolean | undefined);
/**
* This signature is preferred for logging capability.
*/
public constructor(message: string, options: ProviderErrorOptionsType);
public constructor(message: string, options: boolean | ProviderErrorOptionsType = true) {
let logger: Logger | undefined;
let tryNextLink: boolean = true;

if (typeof options === "boolean") {
logger = undefined;
tryNextLink = options;
} else if (options != null && typeof options === "object") {
logger = options.logger;
tryNextLink = options.tryNextLink ?? true;
}
super(message);
// Remove once we stop targetting ES5.
this.tryNextLink = tryNextLink;
Object.setPrototypeOf(this, ProviderError.prototype);
logger?.debug?.(`${this.constructor?.name} ${tryNextLink ? "->" : "(!)"} ${message}`);
}
static from(error: Error, tryNextLink = true): ProviderError {
return Object.assign(new this(error.message, tryNextLink), error);

/**
* @deprecated use new operator.
*/
static from(error: Error, options: boolean | ProviderErrorOptionsType = true): ProviderError {
return Object.assign(new this(error.message, options as ProviderErrorOptionsType), error);
}
}
Loading

0 comments on commit 9d8a589

Please sign in to comment.