Skip to content

Conversation

@Hweinstock
Copy link
Contributor

@Hweinstock Hweinstock commented Jan 28, 2025

Problem

#6451

This rename test relies on a specific result of a race condition for the expected result.

  • The test is checking for a telemetry result that is only emitted when fs.exists takes more than 1 attempt to resolve to true.
  • Therefore, it wants the first fs.exists check to fail, then a subsequent one to succeed.
  • It does this by not awaiting the result, and then writing the file to be renamed.

Usually this is fine, but it is possible that the write (toFile) happens before the read (fs.exists) since neither is awaited. This behavior leads to a flaky test as described in the issue.

Solution

  • use a stub to force the first call to fs.exists to fail.
  • allow all other calls to "go through" to the original function.

  • Treat all work as PUBLIC. Private feature/x branches will not be squash-merged at release time.
  • Your code changes must meet the guidelines in CONTRIBUTING.md.
  • License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

@Hweinstock Hweinstock marked this pull request as ready for review January 28, 2025 20:39
@Hweinstock Hweinstock requested a review from a team as a code owner January 28, 2025 20:39
Copy link
Contributor

@justinmk3 justinmk3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great explanation!

@justinmk3 justinmk3 merged commit 30face9 into aws:master Jan 30, 2025
26 checks passed
@Hweinstock Hweinstock deleted the flakyTest/renameRaceCondition branch January 30, 2025 18:49
s7ab059789 pushed a commit to s7ab059789/aws-toolkit-vscode that referenced this pull request Feb 19, 2025
…6452

## Problem
aws#6451

This rename test relies on a specific result of a race condition for the
expected result.
- The test is checking for a telemetry result that is only emitted when
`fs.exists` takes more than 1 attempt to resolve to true.
- Therefore, it wants the first `fs.exists` check to fail, then a
subsequent one to succeed.
- It does this by not awaiting the result, and then writing the file to
be renamed.

Usually this is fine, but it is possible that the write (`toFile`)
happens before the read (`fs.exists`) since neither is awaited. This
behavior leads to a flaky test as described in the issue.

## Solution
- use a stub to force the first call to `fs.exists` to fail. 
- allow all other calls to "go through" to the original function.
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.

3 participants