Skip to content

Conversation

@Hweinstock
Copy link
Contributor

@Hweinstock Hweinstock commented Sep 24, 2024

Problem

The test case here: https://github.com/aws/aws-toolkit-vscode/blob/359b1d8252a32e064482678110c5931630bd49e0/packages/core/src/test/awsService/ec2/model.test.ts#L132C9-L146C11

This test is unaware of the side-effect that it will attempt to explicitly delete the keys when generating. Originally, the delete was conditional on the keys existing in the directory, so this test case was not a problem since they never existed. However, on windows that check was faulty, leading to the case where the windows file system reported that the keys existed, which lead it to attempting to delete the root directory.

Solution

  • Modify tryRun to accept an onStdOut optional parameter that lets us accept the overwrite prompt from ssh-keygen.
  • Do not explicitly delete the files in the generation function, make this part of the generation step.

The benefits of this approach are that we moved the side-effects to a place where they are expected, in the key generation process itself. It also inherently avoids the potentially faulty check if the key files exist, as we only use onStdOut if when the keys exist.

Notes

  • packages/core/src/awsService/ec2/sshKeyPair.ts:49 includes any which should be ChildProcess. For some reason the import fails in CI, so I left it as any. Refer to attempt to import child process commit for logs.

License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions
Copy link

This pull request implements a feature or fix, so it must include a changelog entry. See CONTRIBUTING.md#changelog for instructions.

@Hweinstock Hweinstock changed the title fix(ec2): fix flaky windows test. test(ec2): fix flaky windows test. Sep 24, 2024
@Hweinstock Hweinstock marked this pull request as ready for review September 26, 2024 20:06
@Hweinstock Hweinstock requested a review from a team as a code owner September 26, 2024 20:06
@Hweinstock
Copy link
Contributor Author

/retryBuilds

@Hweinstock Hweinstock merged commit 27f869b into aws:master Oct 1, 2024
9 of 10 checks passed
@Hweinstock Hweinstock deleted the fixWindowsTest branch October 1, 2024 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants