Skip to content

Speed up directory monitoring tests #1325

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Speed up directory monitoring tests #1325

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions Tests/SwiftDocCUtilitiesTests/DirectoryMonitorTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ class DirectoryMonitorTests: XCTestCase {
}
}

/// - Warning: Please do not overuse this method as it takes 10s of wait time and can potentially slow down running the test suite.
/// - Warning: Please do not overuse this method as it takes 2 seconds of
/// wait time per invocation, and can potentially slow down the test suite.
private func monitorNoUpdates(url: URL, testBlock: @escaping () throws -> Void, file: StaticString = #filePath, line: UInt = #line) throws {
let monitor = try DirectoryMonitor(root: url) { rootURL, url in
XCTFail("Did produce file update event for a hidden file", file: file, line: line)
Expand All @@ -104,12 +105,15 @@ class DirectoryMonitorTests: XCTestCase {
try? testBlock()
}

// For the test purposes we assume a file change event will be delivered within generous 10 seconds.
DispatchQueue.global().asyncAfter(deadline: .now() + 10) {
// For the test purposes we assume a file change event will be delivered within 1.5 seconds.
// This also aligns with the `monitor` method above, that ensures that file change events
// in tests are received within 1.5 seconds. If this works too eagerly, then the other tests
// in this suite will fail.
DispatchQueue.global().asyncAfter(deadline: .now() + 1.5) {
didNotTriggerUpdateForHiddenFile.fulfill()
}

wait(for: [didNotTriggerUpdateForHiddenFile], timeout: 20)
wait(for: [didNotTriggerUpdateForHiddenFile], timeout: 2)
Copy link
Contributor

@d-ronnqvist d-ronnqvist Oct 31, 2025

Choose a reason for hiding this comment

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

This also doesn't speed up the test as long as it passes. Even if I increase both of these to 200 second testMonitorDoesNotTriggerUpdates only takes ~20 seconds for me to run locally.

Copy link
Contributor Author

@snprajwal snprajwal Nov 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, the speed-up comes from reducing the timeout for .fulfill() a few lines above. Since the fulfillment happens in 1.5s, this wait() will always complete shortly after that duration, irrespective of whether the timeout is 2s or 20s. I reduced it to 2s just so it's clear that it waits slightly longer than the time taken for .fulfill() to run. I'm happy to revert it to 20s if you prefer that, but I don't see how that makes the test any less fragile.

}
#endif

Expand Down