(fix): Fixed flaky test TestReadDirSymlink that frequently failed in CI environments due to platform-specific filesystem issues.

[XuPeng-SH]

User description

What type of PR is this?

• API-change
• BUG
• Improvement
• Documentation
• Feature
• Test and CI
• Code Refactoring

Which issue(s) this PR fixes:

issue #22009

What this PR does / why we need it:

Problem

The test was failing with high probability in CI with errors like:
FSEventStreamStart errors on macOS
Path mismatches due to symlink resolution (/var vs /private/var)
panic: index out of range [0] when filesystem watching failed

Root Causes

Symlink path inconsistency: macOS resolves /var to /private/var, causing path comparison failures
Filesystem watching failures: notify.Watch() can fail on macOS, Docker containers, and restricted CI environments
Temporary directory paths: os.MkdirTemp() may return paths containing symlinks that need normalization

Changes

Normalize root path with filepath.EvalSymlinks() at test start
Make filesystem watching failures non-fatal (changed from assert to warning log)
Fix symlink comparison by using EvalSymlinks on both sides
Handle nil event channel gracefully

---

PR Type

Tests, Bug fix

---

Description

• Normalize temporary directory path using filepath.EvalSymlinks() to handle platform-specific symlink resolution (e.g., /var → /private/var on macOS)

• Make filesystem watching failures non-fatal by logging warnings instead of failing test assertions

• Handle nil event channel gracefully when notify.Watch() fails in restricted CI environments

• Fix symlink comparison by applying filepath.EvalSymlinks() to both expected and actual paths

---

Diagram Walkthrough

flowchart LR
  A["Test Start"] --> B["Normalize root path<br/>with EvalSymlinks"]
  B --> C["Attempt notify.Watch"]
  C --> D{Watch succeeds?}
  D -->|Yes| E["Monitor filesystem<br/>events"]
  D -->|No| F["Log warning<br/>continue test"]
  E --> G["Compare symlinks<br/>with EvalSymlinks"]
  F --> G
  G --> H["Test Complete"]

Loading

File Walkthrough

Relevant files

Bug fix

external_test.go Normalize symlinks and handle watch failures gracefully    pkg/sql/colexec/external/external_test.go Added filepath.EvalSymlinks() call on temporary directory root to normalize symlink paths Changed notify.Watch() error handling from assertion to warning log for non-fatal failures Added nil check for event channel before starting filesystem monitoring goroutine Applied filepath.EvalSymlinks() to both sides of symlink path comparison to ensure consistent resolution +20/-4

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Test reliability risk Description: Swallowing notify.Watch errors and continuing the test may mask real regressions by not validating filesystem watching behavior, reducing test effectiveness. external_test.go [403-411] Referred Code // File system watching may fail on some platforms or in CI environments, so we don't fail the test if err != nil { t.Logf("Warning: notify.Watch failed (non-fatal): %v", err) evChan = nil } if evChan != nil { defer notify.Stop(evChan) }
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Test Logging Scope: The added test logging (warnings and notify event logs) does not pertain to production audit trails, so compliance cannot be assessed based on this diff alone. Referred Code // File system watching may fail on some platforms or in CI environments, so we don't fail the test if err != nil { t.Logf("Warning: notify.Watch failed (non-fatal): %v", err) evChan = nil } if evChan != nil { defer notify.Stop(evChan) } testDone := make(chan struct{}) fsLogDone := make(chan struct{}) go func() { defer func() { close(fsLogDone) }() if evChan == nil { return } for { select { case ev := <-evChan: ... (clipped 3 lines)
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Test Input Scope: Changes only affect test behavior (symlink normalization and watcher handling), so validation and data handling of external inputs in production code cannot be evaluated from this diff. Referred Code // Resolve root to its canonical path to handle symlinks (e.g., /var -> /private/var on macOS) root, err = filepath.EvalSymlinks(root) assert.Nil(t, err) t.Cleanup(func() { _ = os.RemoveAll(root) }) evChan := make(chan notify.EventInfo, 1024) err = notify.Watch(filepath.Join(root, "..."), evChan, notify.All) // File system watching may fail on some platforms or in CI environments, so we don't fail the test if err != nil { t.Logf("Warning: notify.Watch failed (non-fatal): %v", err) evChan = nil } if evChan != nil { defer notify.Stop(evChan) } testDone := make(chan struct{}) fsLogDone := make(chan struct{}) ... (clipped 56 lines)

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

No code suggestions found for the PR.