Skip to content

Fix flaky FileIOTest.testMatchWatchForNewFiles test under CI filesystems #38047

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
bvolpato wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix flaky FileIOTest.testMatchWatchForNewFiles test under CI filesystems #38047

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5e86d7e
Fix flaky FileIOTest.testMatchWatchForNewFiles test under CI filesystems
bvolpato Apr 2, 2026
9aa0988
Stabilize FileIOTest updated-file timestamp assertions
bvolpato May 5, 2026
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
11 changes: 10 additions & 1 deletion sdks/java/core/src/test/java/org/apache/beam/sdk/io/FileIOTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,16 +249,25 @@ public void processElement(ProcessContext context, @StateId("count") ValueState<
context.output(Objects.requireNonNull(context.element()).getValue());

CopyOption[] cpOptions = {StandardCopyOption.COPY_ATTRIBUTES};
CopyOption[] updOptions = {StandardCopyOption.REPLACE_EXISTING};
CopyOption[] updOptions = {
StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.COPY_ATTRIBUTES
};
final Path sourcePath = Paths.get(sourcePathStr);
final Path watchPath = Paths.get(watchPathStr);

if (0 == current) {
Thread.sleep(100);
// Ensure overwrite updates get a distinct mtime even when COPY_ATTRIBUTES is enabled.
Files.setLastModifiedTime(
sourcePath.resolve("first"), FileTime.fromMillis(System.currentTimeMillis() + 2000));
Comment on lines +261 to +262
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 5, 2026

Choose a reason for hiding this comment

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

medium

Using a future timestamp (System.currentTimeMillis() + 2000) is a workaround for filesystem resolution issues but can be fragile in environments that validate file timestamps. Consider if a more deterministic way to ensure increasing timestamps (e.g., using a base time and incrementing) would be more reliable.

Files.copy(sourcePath.resolve("first"), watchPath.resolve("first"), updOptions);
Files.copy(sourcePath.resolve("second"), watchPath.resolve("second"), cpOptions);
} else if (1 == current) {
Thread.sleep(100);
Files.setLastModifiedTime(
sourcePath.resolve("first"), FileTime.fromMillis(System.currentTimeMillis() + 4000));
Files.setLastModifiedTime(
sourcePath.resolve("second"), FileTime.fromMillis(System.currentTimeMillis() + 4000));
Comment on lines +267 to +270
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 5, 2026

Choose a reason for hiding this comment

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

medium

Calling System.currentTimeMillis() multiple times for related updates can lead to inconsistent timestamps if the clock ticks between calls. It's better to capture the timestamp once. Also, the use of future offsets (e.g., + 4000) remains a potential source of flakiness on some platforms.

Suggested change
Files.setLastModifiedTime(
sourcePath.resolve("first"), FileTime.fromMillis(System.currentTimeMillis() + 4000));
Files.setLastModifiedTime(
sourcePath.resolve("second"), FileTime.fromMillis(System.currentTimeMillis() + 4000));
long now = System.currentTimeMillis();
Files.setLastModifiedTime(
sourcePath.resolve("first"), FileTime.fromMillis(now + 4000));
Files.setLastModifiedTime(
sourcePath.resolve("second"), FileTime.fromMillis(now + 4000));

Files.copy(sourcePath.resolve("first"), watchPath.resolve("first"), updOptions);
Files.copy(sourcePath.resolve("second"), watchPath.resolve("second"), updOptions);
Files.copy(sourcePath.resolve("third"), watchPath.resolve("third"), cpOptions);
Expand Down
Loading