Fix flaky workloadIdentityCredentialIdentityBinding test on Windows (#36745)

Copilot · minhanh-phan · web-flow · commit 854d41e4a6d5 · 2025-12-09T10:47:19.000-08:00
### Packages impacted by this PR - @azure/identity ### Issues associated with this PR - #33623 ### Describe the problem that is addressed by this PR Test `should use custom token endpoint with CA file configuration` fails intermittently on Windows with `ENOTEMPTY: directory not empty, rmdir`. The "Certificate Validation & Processing" tests used `fs.unlink()` + `fs.rmdir()` for temp directory cleanup, which fails on Windows due to timing issues with file handle releases. Additionally, the inline cleanup approach meant that cleanup would not occur if test assertions failed. ### What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen? Two main approaches were implemented: 1. **Replace two-step cleanup pattern**: Changed from `fs.unlink()` + `fs.rmdir()` to `fs.rm(tempDir, { recursive: true, force: true })` for reliable cross-platform directory cleanup. This pattern is already used elsewhere in this test file ("Configuration validation" and "Identity binding feature" sections). 2. **Refactor cleanup to afterEach block**: Moved cleanup logic to an `afterEach` hook to ensure cleanup happens even if test assertions fail. This involved: - Declaring `tempDir` and `tempCaFile` as describe-block-level variables typed as `string | undefined` - Adding error handling in the cleanup to prevent test suite failures from filesystem issues - Resetting variables to `undefined` after cleanup to prevent stale path reuse This combined approach ensures both reliable cleanup on Windows and guaranteed cleanup regardless of test outcome. ### Are there test cases added in this PR? _(If not, why?)_ No new tests needed—this fixes existing test cleanup reliability and robustness. ### Provide a list of related PRs _(if any)_ N/A ### Command used to generate this PR:**_(Applicable only to SDK release request PRs)_ N/A ### Checklists - [x] Added impacted package name to the issue description - [ ] Does this PR needs any fixes in the SDK Generator?** _(If so, create an Issue in the [Autorest/typescript](https://github.com/Azure/autorest.typescript) repository and link it here)_ - [ ] Added a changelog (if necessary)  <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>[test] [identity] flaky workloadIdentityCredentialIdentityBinding test?</issue_title> > <issue_description>```text > @azure/identity:test:node: [vitest] ⎯⎯⎯⎯⎯⎯⎯ Failed Tests 1 ⎯⎯⎯⎯⎯⎯⎯ > @azure/identity:test:node: [vitest] > @azure/identity:test:node: [vitest] FAIL test/internal/node/workloadIdentityCredentialIdentityBinding.spec.ts > WorkloadIdentityCredential - Identity Binding Configuration > Identity binding feature > should use custom token endpoint with CA file configuration > @azure/identity:test:node: [vitest] Error: ENOTEMPTY: directory not empty, rmdir 'C:\Users\CLOUDT~1\AppData\Local\Temp\tempDir3YnkHc' > @azure/identity:test:node: [vitest] ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯ > @azure/identity:test:node: [vitest] > @azure/identity:test:node: [vitest] > @azure/identity:test:node: [vitest] Test Files 1 failed | 38 passed | 3 skipped (42) > @azure/identity:test:node: [vitest] Tests 1 failed | 384 passed | 17 skipped (402) > @azure/identity:test:node: [vitest] Start at 19:35:52 > @azure/identity:test:node: [vitest] Duration 27.69s (transform 12.00s, setup 0ms, collect 37.61s, tests 21.64s, environment 14ms, prepare 816ms) > ```</issue_description> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > <comment_new><author>@jeremymeng</author><body> > https://dev.azure.com/azure-sdk/public/public%20Team/_build/results?buildId=5605868&view=logs&j=3e883751-031e-56f9-9938-d1ed90b686e8&t=209f8deb-5df3-5fcd-d026-4e85ab01a9fe&l=2126</body></comment_new> > </comments> > </details> - Fixes #36735  --- ✨ Let Copilot coding agent [set things up for you](https://github.com/Azure/azure-sdk-for-js/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: minhanh-phan <111523473+minhanh-phan@users.noreply.github.com>
diff --git a/sdk/identity/identity/test/internal/node/workloadIdentityCredentialIdentityBinding.spec.ts b/sdk/identity/identity/test/internal/node/workloadIdentityCredentialIdentityBinding.spec.ts
@@ -37,6 +37,22 @@ describe("WorkloadIdentityCredential - Identity Binding Configuration", function
   });
 
   describe("Certificate Validation & Processing", function () {
+    let tempDir: string | undefined;
+    let tempCaFile: string | undefined;
+
+    afterEach(async function () {
+      if (tempDir) {
+        try {
+          await fs.rm(tempDir, { recursive: true, force: true });
+        } catch (error) {
+          // Ignore cleanup errors to prevent test suite failures
+        } finally {
+          tempDir = undefined;
+          tempCaFile = undefined;
+        }
+      }
+    });
+
     it("should throw error for invalid CA certificate data", async function () {
       vi.stubEnv("AZURE_KUBERNETES_TOKEN_PROXY", "https://test-proxy.example.com");
       vi.stubEnv("AZURE_KUBERNETES_CA_DATA", "invalid-certificate-data");
@@ -52,8 +68,8 @@ describe("WorkloadIdentityCredential - Identity Binding Configuration", function
     });
     it("should validate CA file changes and cache invalidation", async function () {
       const invalidCaContent = "invalid-certificate-data";
-      const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "cert-test-"));
-      const tempCaFile = path.join(tempDir, "ca.pem");
+      tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "cert-test-"));
+      tempCaFile = path.join(tempDir, "ca.pem");
       // Copy valid certificate initially
       await fs.copyFile(TEST_CERT_PATH, tempCaFile);
 
@@ -100,13 +116,11 @@ describe("WorkloadIdentityCredential - Identity Binding Configuration", function
 
       // Should be a new object reference since cache was invalidated
       assert.equal(tlsSettings3.ca, getTestCertificateContent());
-      await fs.unlink(tempCaFile);
-      await fs.rmdir(tempDir);
     });
 
     it("should handle empty CA file during rotation", async function () {
-      const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "cert-test-"));
-      const tempCaFile = path.join(tempDir, "ca.pem");
+      tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "cert-test-"));
+      tempCaFile = path.join(tempDir, "ca.pem");
 
       await fs.copyFile(TEST_CERT_PATH, tempCaFile);
 
@@ -143,9 +157,6 @@ describe("WorkloadIdentityCredential - Identity Binding Configuration", function
           enableAzureProxy: true,
         });
       }, /CA certificate file is empty/);
-
-      await fs.unlink(tempCaFile);
-      await fs.rmdir(tempDir);
     });
   });
 
