test(policychecks): Implement IamPolicyChecks unit tests #5570
Conversation
kevluu-aws
changed the title
packages/core/src/awsService/accessanalyzer/vue/iamPolicyChecks.ts
Outdated
Show resolved
Hide resolved
packages/core/src/test/awsService/accessanalyzer/iamPolicyChecks.test.ts
Outdated
Show resolved
Hide resolved
packages/core/src/test/awsService/accessanalyzer/iamPolicyChecks.test.ts
Outdated
Show resolved
Hide resolved
packages/core/src/test/awsService/accessanalyzer/iamPolicyChecks.test.ts
Outdated
Show resolved
Hide resolved
| executeCustomPolicyChecksCommandStub.calledOnceWith({ | ||
| command: 'cfn-policy-validator', |
There was a problem hiding this comment.
can you download the actual tool in CI and actually invoke it, instead of stubbing this?
There was a problem hiding this comment.
I don't think we want to call the actual tool as it makes calls to an actual AWS service and requires credentials. Do we have a set of credentials we use for testing AWS services here? If we do, I think we can do it; the test will depend on the availability of the service in that case.
There was a problem hiding this comment.
currently our CI only has a Builder Id token, not IAM credentials
kevluu-aws
force-pushed
the
tests
branch
12 times, most recently
from
f7349db
to
d525029
Compare
Why is |
| } catch (err) { | ||
| logger.error(err as Error) | ||
| } | ||
| } | ||
|
|
||
| // Helper function to get document contents from a path | ||
| async function _readCustomChecksFile(input: string): Promise<string> { | ||
| export async function _readCustomChecksFile(input: string): Promise<string> { | ||
| if (fs.existsSync(input)) { | ||
| return fs.readFileSync(input).toString() |
There was a problem hiding this comment.
please migrate the app code to use our fs.ts module, do not use nodefs directly
| import * as iamPolicyChecks from '../../../awsService/accessanalyzer/vue/iamPolicyChecks' | ||
| import * as vscode from 'vscode' | ||
| import { IamPolicyChecksConstants } from '../../../awsService/accessanalyzer/vue/constants' | ||
| import { FileSystem } from '../../../shared/fs/fs' |
There was a problem hiding this comment.
| import { FileSystem } from '../../../shared/fs/fs' | |
| import fs from '../../../shared/fs/fs' |
Problem
IamPolicyChecks currently has no unit tests. This makes the feature fragile to changes and at risk to breaking changes.
Solution
Implement unit tests for IamPolicyChecks feature.
Note: Functions that require
execFileSyncwere skipped as it was not possible to stub the execution of the underlying CLI tools used.License: I confirm that my contribution is made under the terms of the Apache 2.0 license.