From dea07d624d5a4d9847ad04a8fc5bf86d076b70e8 Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 28 Jan 2025 15:04:25 -0500 Subject: [PATCH 1/3] fix: use stub to avoid race condition --- packages/core/src/shared/fs/fs.ts | 5 +++++ packages/core/src/test/shared/fs/fs.test.ts | 13 ++++++++----- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/packages/core/src/shared/fs/fs.ts b/packages/core/src/shared/fs/fs.ts index 1c7132648bc..d1e4d93d7c9 100644 --- a/packages/core/src/shared/fs/fs.ts +++ b/packages/core/src/shared/fs/fs.ts @@ -312,6 +312,11 @@ export class FileSystem { reasonDesc: `After multiple attempts the source path existed: ${scrubbedPath}`, attempts: attempts, }) + } else { + telemetry.ide_fileSystem.emit({ + result: 'Succeeded', + action: 'rename', + }) } return vfs.rename(oldUri, newUri, { overwrite: true }).then(undefined, errHandler) diff --git a/packages/core/src/test/shared/fs/fs.test.ts b/packages/core/src/test/shared/fs/fs.test.ts index c816ecdde83..15ee7e56fd5 100644 --- a/packages/core/src/test/shared/fs/fs.test.ts +++ b/packages/core/src/test/shared/fs/fs.test.ts @@ -411,16 +411,19 @@ describe('FileSystem', function () { const oldPath = testFolder.pathFrom('oldFile.txt') const newPath = testFolder.pathFrom('newFile.txt') - const result = fs.rename(oldPath, newPath) - // this file is created after the first "exists" check fails, the following check should pass - void testutil.toFile('hello world', oldPath) - await result + const existsStub = Sinon.stub(FileSystem.prototype, 'exists') + existsStub.onFirstCall().resolves(false) + existsStub.callThrough() + + await testutil.toFile('hello world', oldPath) + await fs.rename(oldPath, newPath) testutil.assertTelemetry('ide_fileSystem', { action: 'rename', result: 'Succeeded', - reason: 'RenameRaceCondition', }) + + existsStub.restore() }) }) From 186570f34ac09e8630fa97076ccc0729941d1fa1 Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 28 Jan 2025 15:18:24 -0500 Subject: [PATCH 2/3] test: add back expected telemetry metadata --- packages/core/src/test/shared/fs/fs.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/core/src/test/shared/fs/fs.test.ts b/packages/core/src/test/shared/fs/fs.test.ts index 15ee7e56fd5..15db2c6b73e 100644 --- a/packages/core/src/test/shared/fs/fs.test.ts +++ b/packages/core/src/test/shared/fs/fs.test.ts @@ -421,6 +421,7 @@ describe('FileSystem', function () { testutil.assertTelemetry('ide_fileSystem', { action: 'rename', result: 'Succeeded', + reason: 'RenameRaceCondition', }) existsStub.restore() From c8707d7c813e5895d97f92aaa2a956215a265498 Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 28 Jan 2025 15:28:20 -0500 Subject: [PATCH 3/3] telemetry: remove default emit --- packages/core/src/shared/fs/fs.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/core/src/shared/fs/fs.ts b/packages/core/src/shared/fs/fs.ts index d1e4d93d7c9..1c7132648bc 100644 --- a/packages/core/src/shared/fs/fs.ts +++ b/packages/core/src/shared/fs/fs.ts @@ -312,11 +312,6 @@ export class FileSystem { reasonDesc: `After multiple attempts the source path existed: ${scrubbedPath}`, attempts: attempts, }) - } else { - telemetry.ide_fileSystem.emit({ - result: 'Succeeded', - action: 'rename', - }) } return vfs.rename(oldUri, newUri, { overwrite: true }).then(undefined, errHandler)