Skip to content

Fix flaky workloadIdentityCredentialIdentityBinding test on Windows #36745

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix flaky workloadIdentityCredentialIdentityBinding test on Windows #36745

wants to merge 2 commits into from

Conversation

Copy link
Contributor

Copilot AI commented Nov 25, 2025 •
edited
Loading

Packages impacted by this PR

  • @azure/identity

Issues associated with this PR

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.

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?

Replace the two-step cleanup with fs.rm(tempDir, { recursive: true, force: true }). This is the same pattern already used elsewhere in this test file ("Configuration validation" and "Identity binding feature" sections).

Are there test cases added in this PR? (If not, why?)

No new tests needed—this fixes existing test cleanup reliability.

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

  • 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 repository and link it here)
  • Added a changelog (if necessary)

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • login.microsoftonline.com
    • Triggering command: /opt/hostedtoolcache/node/20.19.5/x64/bin/node /opt/hostedtoolcache/node/20.19.5/x64/bin/node --conditions node --conditions development --experimental-import-meta-resolve --require /home/REDACTED/work/azure-sdk-for-js/azure-sdk-for-js/node_modules/.pnpm/[email protected]_@opentelemetry+[email protected]_@types+[email protected]_@types+[email protected]_@vitest_4be2d1158f082e5fc4d55edbc8c2e690/node_modules/vitest/suppress-warnings.cjs /home/REDACTED/work/azure-sdk-for-js/azure-sdk-for-js/node_modules/.pnpm/[email protected]_@opentelemetry+[email protected]_@types+[email protected]_@types+[email protected]_@vitest_4be2d1158f082e5fc4d55edbc8c2e690/node_modules/vitest/dist/workers/forks.js (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Original prompt

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)


## 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>

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@minhanh-phan
Fix flaky workloadIdentityCredentialIdentityBinding test cleanup
bba3360 
Replace fs.unlink + fs.rmdir with fs.rm for reliable directory cleanup on Windows

Co-authored-by: minhanh-phan <[email protected]>
Copilot AI changed the title [WIP] Fix flaky WorkloadIdentityCredentialIdentityBinding test Fix flaky workloadIdentityCredentialIdentityBinding test on Windows Nov 25, 2025
Copilot AI requested a review from minhanh-phan November 25, 2025 18:26
Copilot finished work on behalf of minhanh-phan November 25, 2025 18:26
@minhanh-phan minhanh-phan marked this pull request as ready for review November 25, 2025 19:04
Copilot AI review requested due to automatic review settings November 25, 2025 19:04
Copilot finished reviewing on behalf of minhanh-phan November 25, 2025 19:05
Copilot AI reviewed Nov 25, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a Windows-specific flaky test failure in the @azure/identity package. The test should use custom token endpoint with CA file configuration was intermittently failing on Windows with ENOTEMPTY: directory not empty, rmdir errors during cleanup. The fix standardizes the directory cleanup approach to use the more robust fs.rm() method instead of the two-step fs.unlink() + fs.rmdir() pattern.

Key changes:

  • Replaces two-step cleanup (fs.unlink() + fs.rmdir()) with single-step fs.rm() using recursive and force options
  • Applied to two test cases in the "Certificate Validation & Processing" section

@minhanh-phan
Copy link
Member

minhanh-phan commented Nov 25, 2025

Pipelines run success here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@minhanh-phan minhanh-phan Awaiting requested review from minhanh-phan minhanh-phan is a code owner

@KarishmaGhiya KarishmaGhiya Awaiting requested review from KarishmaGhiya KarishmaGhiya is a code owner

@maorleger maorleger Awaiting requested review from maorleger maorleger is a code owner

@joshfree joshfree Awaiting requested review from joshfree joshfree is a code owner automatically assigned from Azure/azure-sdk-write-identity

@pvaneck pvaneck Awaiting requested review from pvaneck pvaneck is a code owner automatically assigned from Azure/azure-sdk-write-identity

@JonathanCrd JonathanCrd Awaiting requested review from JonathanCrd JonathanCrd is a code owner automatically assigned from Azure/azure-sdk-write-identity

@anannya03 anannya03 Awaiting requested review from anannya03 anannya03 is a code owner automatically assigned from Azure/azure-sdk-write-identity

At least 1 approving review is required to merge this pull request.

Assignees

@minhanh-phan minhanh-phan

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[test] [identity] flaky workloadIdentityCredentialIdentityBinding test?

2 participants

@minhanh-phan