Skip to content

[microsoft.azd.extensions] Fix azd x watch glob matching on Windows and enable CI tests#8278

Merged
JeffreyCA merged 2 commits into
Azure:mainfrom
JeffreyCA:fix/azd-x-watch-windows-glob-match
May 20, 2026
Merged

[microsoft.azd.extensions] Fix azd x watch glob matching on Windows and enable CI tests#8278
JeffreyCA merged 2 commits into
Azure:mainfrom
JeffreyCA:fix/azd-x-watch-windows-glob-match

Conversation

@JeffreyCA

@JeffreyCA JeffreyCA commented May 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #8273. Two related changes:

  1. Fix glob matching on Windows. The hardcoded ignore fast-path in shouldIgnoreWatchEvent was using doublestar.PathMatch, which splits patterns on the OS path separator. On Windows that separator is \, so multi-segment patterns like bin/**/* could never match the forward-slashed relPath produced by filepath.ToSlash. Switched to doublestar.Match, which always splits on / and matches the convention already in use for relPath and the hardcoded patterns. This is the library's documented recommendation when both inputs are forward-slashed.

  2. Enable unit tests in CI for microsoft.azd.extensions. The pipeline had SkipTests: true and the extension was missing ci-test.ps1. Without these, the existing TestShouldIgnoreWatchEvent* tests (and any future tests) never run in CI, so Windows-only regressions like (1) can slip through. Mirrors the setup used by other extensions (e.g. azure.ai.agents).

User impact of the matching bug

In practice on Windows the broken matcher was largely dormant:

  • watchRecursive skips ignored folders by directory name at any depth via filepath.SkipDir, so bin/, obj/, build/, node_modules/, and .git/ are never added to the watcher.
  • fsnotify on Windows is non-recursive, so events for files inside non-added directories never surface to shouldIgnoreWatchEvent.
  • Single-segment patterns (bin, *.spec, package-lock.json, ...) still matched correctly even with the OS-separator bug.

The fix removes a latent footgun and makes the existing TestShouldIgnoreWatchEvent* unit tests pass on Windows. Linux and macOS were unaffected.

Verification

cd cli/azd/extensions/microsoft.azd.extensions
go test ./internal/cmd/...
# ok  github.com/azure/azure-dev/cli/azd/extensions/microsoft.azd.extensions/internal/cmd

pwsh ./ci-test.ps1
# All tests passed!

shouldIgnoreWatchEvent was using doublestar.PathMatch, which splits patterns on the OS separator. On Windows that's '\\', so multi-segment patterns like `bin/**/*` could not match the forward-slashed relPath produced by filepath.ToSlash, defeating the hardcoded ignore fast-path.

Switch to doublestar.Match (always splits on '/'), matching the convention already used for relPath and the hardcoded patterns. This is the library's documented recommendation when both inputs are forward-slashed.

Fixes the two TestShouldIgnoreWatchEvent* tests that were failing on Windows.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes Windows-specific glob matching in azd x watch by aligning doublestar glob evaluation with the forward-sash (/) normalization already used for watcher event relative paths.

Changes:

  • Replace doublestar.PathMatch with doublestar.Match in shouldIgnoreWatchEvent to correctly match multi-segment patterns like bin/**/* on Windows.
  • Add an inline comment explaining why Match is required given relativeWatchPath returns forward-slashed paths.

Remove SkipTests: true from the extension's release pipeline and add a ci-test.ps1 mirroring the script used by other extensions (e.g. azure.ai.agents). Without this, the TestShouldIgnoreWatchEvent* tests (and any future tests) never run in CI, so Windows-only regressions like the doublestar.PathMatch issue fixed in the preceding commit can slip through.
@JeffreyCA
JeffreyCA requested a review from danieljurek as a code owner May 20, 2026 21:22
@JeffreyCA JeffreyCA changed the title [microsoft.azd.extensions] Fix azd x watch glob matching on Windows [microsoft.azd.extensions] Fix azd x watch glob matching on Windows and enable CI tests May 20, 2026
@github-actions github-actions Bot added the ext-x microsoft.azd.extensions developer extension (azd x) label May 20, 2026

@hemarina hemarina left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM — no blocking or non-blocking issues found.

The Windows glob matching fix uses the correct doublestar API for forward-slashed relative paths, and the CI test enablement follows the existing extension pipeline pattern.

@JeffreyCA
JeffreyCA enabled auto-merge (squash) May 20, 2026 22:23
@JeffreyCA

Copy link
Copy Markdown
Contributor Author

/check-enforcer override

@JeffreyCA
JeffreyCA merged commit bfdf8e9 into Azure:main May 20, 2026
22 of 23 checks passed
@JeffreyCA
JeffreyCA deleted the fix/azd-x-watch-windows-glob-match branch May 20, 2026 22:27
Copilot AI pushed a commit that referenced this pull request May 21, 2026
…nd enable CI tests (#8278)

* [microsoft.azd.extensions] Fix azd x watch glob matching on Windows

shouldIgnoreWatchEvent was using doublestar.PathMatch, which splits patterns on the OS separator. On Windows that's '\\', so multi-segment patterns like `bin/**/*` could not match the forward-slashed relPath produced by filepath.ToSlash, defeating the hardcoded ignore fast-path.

Switch to doublestar.Match (always splits on '/'), matching the convention already used for relPath and the hardcoded patterns. This is the library's documented recommendation when both inputs are forward-slashed.

Fixes the two TestShouldIgnoreWatchEvent* tests that were failing on Windows.

* [microsoft.azd.extensions] Enable unit tests in CI

Remove SkipTests: true from the extension's release pipeline and add a ci-test.ps1 mirroring the script used by other extensions (e.g. azure.ai.agents). Without this, the TestShouldIgnoreWatchEvent* tests (and any future tests) never run in CI, so Windows-only regressions like the doublestar.PathMatch issue fixed in the preceding commit can slip through.
Co-authored-by: therealjohn <1501196+therealjohn@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ext-x microsoft.azd.extensions developer extension (azd x)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

azd x watch rebuilds continuously when output directory changes

5 participants