Skip to content

Commit

Permalink
Address review, add test.
Browse files Browse the repository at this point in the history
  • Loading branch information
Cornelius Riemenschneider authored and GitHub committed Jul 18, 2022
1 parent 01fa64c commit 75afbf4
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 20 deletions.
6 changes: 3 additions & 3 deletions lib/feature-flags.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/feature-flags.js.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 16 additions & 0 deletions lib/feature-flags.test.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/feature-flags.test.js.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

47 changes: 39 additions & 8 deletions src/feature-flags.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ const ALL_FEATURE_FLAGS_DISABLED_VARIANTS: Array<{
description: string;
gitHubVersion: util.GitHubVersion;
}> = [
{
description: "GHES",
gitHubVersion: { type: GitHubVariant.GHES, version: "3.0.0" },
},
{ description: "GHAE", gitHubVersion: { type: GitHubVariant.GHAE } },
];
{
description: "GHES",
gitHubVersion: { type: GitHubVariant.GHES, version: "3.0.0" },
},
{ description: "GHAE", gitHubVersion: { type: GitHubVariant.GHAE } },
];

for (const variant of ALL_FEATURE_FLAGS_DISABLED_VARIANTS) {
test(`All feature flags are disabled if running against ${variant.description}`, async (t) => {
Expand All @@ -60,13 +60,44 @@ for (const variant of ALL_FEATURE_FLAGS_DISABLED_VARIANTS) {
(v: LoggedMessage) =>
v.type === "debug" &&
v.message ===
"Not running against github.com. Disabling all feature flags."
"Not running against github.com. Disabling all feature flags."
) !== undefined
);
});
});
}

test("API response missing", async (t) => {
await withTmpDir(async (tmpDir) => {
setupActionsVars(tmpDir, tmpDir);

const loggedMessages = [];
const featureFlags = new GitHubFeatureFlags(
{ type: GitHubVariant.DOTCOM },
testApiDetails,
testRepositoryNwo,
getRecordingLogger(loggedMessages)
);

mockFeatureFlagApiEndpoint(403, {});

for (const flag of Object.values(FeatureFlag)) {
t.assert((await featureFlags.getValue(flag)) === false);
}

for (const featureFlag of ["ml_powered_queries_enabled"]) {
t.assert(
loggedMessages.find(
(v: LoggedMessage) =>
v.type === "debug" &&
v.message ===
`No feature flags API response for ${featureFlag}, considering it disabled.`
) !== undefined
);
}
});
});

test("Feature flags are disabled if they're not returned in API response", async (t) => {
await withTmpDir(async (tmpDir) => {
setupActionsVars(tmpDir, tmpDir);
Expand All @@ -91,7 +122,7 @@ test("Feature flags are disabled if they're not returned in API response", async
(v: LoggedMessage) =>
v.type === "debug" &&
v.message ===
`Feature flag '${featureFlag}' undefined in API response, considering it disabled.`
`Feature flag '${featureFlag}' undefined in API response, considering it disabled.`
) !== undefined
);
}
Expand Down
14 changes: 7 additions & 7 deletions src/feature-flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export class GitHubFeatureFlags implements FeatureFlags {
private apiDetails: GitHubApiDetails,
private repositoryNwo: RepositoryNwo,
private logger: Logger
) {}
) { }

async getValue(flag: FeatureFlag): Promise<boolean> {
const response = await this.getApiResponse();
Expand All @@ -38,14 +38,14 @@ export class GitHubFeatureFlags implements FeatureFlags {
);
return false;
}
const flag_value = response[flag];
if (flag_value === undefined) {
const flagValue = response[flag];
if (flagValue === undefined) {
this.logger.debug(
`Feature flag '${flag}' undefined in API response, considering it disabled.`
);
return false;
}
return flag_value;
return flagValue;
}

private async getApiResponse(): Promise<FeatureFlagsApiResponse> {
Expand All @@ -71,9 +71,9 @@ export class GitHubFeatureFlags implements FeatureFlags {
if (util.isHTTPError(e) && e.status === 403) {
this.logger.warning(
"This run of the CodeQL Action does not have permission to access Code Scanning API endpoints. " +
"As a result, it will not be opted into any experimental features. " +
"This could be because the Action is running on a pull request from a fork. If not, " +
`please ensure the Action has the 'security-events: write' permission. Details: ${e}`
"As a result, it will not be opted into any experimental features. " +
"This could be because the Action is running on a pull request from a fork. If not, " +
`please ensure the Action has the 'security-events: write' permission. Details: ${e}`
);
} else {
// Some feature flags, such as `ml_powered_queries_enabled` affect the produced alerts.
Expand Down

0 comments on commit 75afbf4

Please sign in to comment.