Skip to content

Conversation

@h-a-n-a
Copy link
Contributor

@h-a-n-a h-a-n-a commented Nov 14, 2025

Summary

Pass more native watcher tests.

  1. Introduce last_changed to indicate when the last change was made. This field is monitored by the event aggregation handler. Once timeout, the handler will be triggered, otherwise the thread will be put to sleep for the remaining duration. (This fixed the case of should watch multiple files where aggregation handler is not triggered according to the last modification)
  2. Cases ported: should_not_watch_a_single_ignored_file_glob, should_not_watch_a_single_ignored_file_regexp, should_watch_multiple_files

Related links

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).

@netlify
Copy link

netlify bot commented Nov 14, 2025

Deploy Preview for rspack canceled.

Name Link
🔨 Latest commit 1c5cdd5
🔍 Latest deploy log https://app.netlify.com/projects/rspack/deploys/691713d0a4d02100083747cb

@github-actions github-actions bot added the team The issue/pr is created by the member of Rspack. label Nov 14, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Nov 14, 2025

📦 Binary Size-limit

Comparing 1c5cdd5 to ci(native-watcher): test multiple platforms (#12197) by Hana

❌ Size increased by 1.63KB from 47.45MB to 47.46MB (⬆️0.00%)

@codspeed-hq
Copy link

codspeed-hq bot commented Nov 14, 2025

CodSpeed Performance Report

Merging #12203 will not alter performance

Comparing port-watchpack-tests (1c5cdd5) with main (a85877a)

Summary

✅ 17 untouched

@h-a-n-a h-a-n-a force-pushed the port-watchpack-tests branch from a660330 to 91c78a1 Compare November 14, 2025 11:02
@h-a-n-a h-a-n-a changed the title test(native-watcher): port more native watcher tests fix(native-watcher): fix aggregate handler trigger Nov 14, 2025
@github-actions github-actions bot added the release: bug fix release: bug related release(mr only) label Nov 14, 2025
@h-a-n-a h-a-n-a marked this pull request as ready for review November 14, 2025 11:05
Copilot AI review requested due to automatic review settings November 14, 2025 11:05
Copilot finished reviewing on behalf of h-a-n-a November 14, 2025 11:08
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 the aggregate handler trigger mechanism in the native watcher by introducing a last_changed field to track when the last file change occurred. The aggregate handler now monitors this timestamp and triggers only after the specified timeout has elapsed since the last change, rather than triggering immediately.

Key Changes

  • Introduced last_changed: Arc<RwLock<Option<Instant>>> to track the timestamp of the last file system event
  • Refactored aggregate task to use a polling loop that checks elapsed time since last change
  • Restructured test helpers with new macros and improved error handling
  • Added comprehensive tests for single and multiple file watching scenarios

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.

File Description
crates/rspack_watcher/src/executor.rs Core logic changes: added last_changed field and refactored aggregate timeout handling from event-based to time-based polling
crates/rspack_watcher/tests/watcher.rs New test cases for single file, ignored files (glob/regex), and multiple file watching scenarios
crates/rspack_watcher/tests/helpers/mod.rs Added collect_events (non-blocking), tick_ms, improved error handling, and changed time precision to microseconds
crates/rspack_watcher/tests/helpers/macros.rs Extracted test macros into separate module for better organization

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@h-a-n-a h-a-n-a enabled auto-merge (squash) November 14, 2025 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release: bug fix release: bug related release(mr only) team The issue/pr is created by the member of Rspack.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants