Skip to content

test case that shows the issue #215

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
vorburger wants to merge 3 commits into
base: main
Choose a base branch
from
Open

test case that shows the issue #215

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
759a857
test case that shows the issue
edewit Jun 8, 2022
486b0b4
Merge branch 'main' into issue-95
vorburger Jan 3, 2026
bbcdc23
Fix QuietPeriodListener swallowing subsequent events (issue #95)
vorburger Jan 3, 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
9 changes: 7 additions & 2 deletions src/main/java/ch/vorburger/fswatch/QuietPeriodListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ public class QuietPeriodListener implements Listener {
protected @Nullable Thread thread;
protected volatile boolean sleepAgain;

protected volatile Path lastPath;
protected volatile ChangeKind lastChangeKind;

/**
* Constructor.
* @param quietPeriodInMS the quiet period in milliseconds
Expand All @@ -54,6 +57,8 @@ public QuietPeriodListener(long quietPeriodInMS, Listener listenerToWrap, Except

@Override
public synchronized void onChange(Path path, ChangeKind changeKind) {
this.lastPath = path;
this.lastChangeKind = changeKind;
if (thread != null && thread.isAlive()) {
sleepAgain = true;
//System.out.println("sleepAgain = true");
Expand All @@ -65,7 +70,7 @@ public synchronized void onChange(Path path, ChangeKind changeKind) {
//System.out.println("sleepAgain = false");
Thread.sleep(quietPeriodInMS);
} while (sleepAgain);
delegate.onChange(path, changeKind);
delegate.onChange(lastPath, lastChangeKind);
} catch (Throwable e) {
exceptionHandler.onException(e);
}
Expand All @@ -75,4 +80,4 @@ public synchronized void onChange(Path path, ChangeKind changeKind) {
thread.start();
}
}
}
}
28 changes: 27 additions & 1 deletion src/test/java/ch/vorburger/fswatch/test/DirectoryAndFileWatcherTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@
package ch.vorburger.fswatch.test;

import static com.google.common.base.Charsets.US_ASCII;
import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
import static java.util.concurrent.TimeUnit.SECONDS;
import static org.awaitility.Awaitility.await;
import static org.hamcrest.Matchers.anyOf;
import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
Expand All @@ -35,6 +37,7 @@
import com.google.common.io.MoreFiles;
import java.io.File;
import java.nio.file.FileSystems;
import java.util.concurrent.atomic.AtomicReference;

import org.jspecify.annotations.Nullable;
import org.junit.BeforeClass;
Expand Down Expand Up @@ -224,6 +227,29 @@ public void testDirectoryWatcherListenerExceptionPropagation() throws Throwable
}
}

@Test
public void testOverwriteExistingFile() throws Throwable {
var assertableExceptionHandler = new AssertableExceptionHandler();
final File dir = new File("target/tests/DirectoryWatcherTest/overwrite_existing");
dir.mkdirs();
final File file = File.createTempFile("test", "txt");
final File to = dir.toPath().resolve("test.txt").toFile();
Files.copy(file, to);

@gemini-code-assist gemini-code-assist Bot Jan 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The com.google.common.io.Files.copy(File, File) method is deprecated and scheduled for removal. It's better to use the standard java.nio.file.Files.copy() instead. This also improves consistency with other parts of the new test that use java.nio.file.Files.

Suggested change
Files.copy(file, to);
java.nio.file.Files.copy(file.toPath(), to.toPath());

Copilot AI Jan 3, 2026

Copy link

Choose a reason for hiding this comment

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

The Guava Files.copy method signature for copying between two File objects requires a target file that doesn't exist, or it will throw an exception. Since this test is intended to demonstrate overwriting an existing file, this line may cause the test to fail. Consider using java.nio.file.Files.copy with appropriate options, or using Files.asCharSink to create the initial file content.

Suggested change
Files.copy(file, to);
java.nio.file.Files.copy(file.toPath(), to.toPath(), REPLACE_EXISTING);

Copilot uses AI. Check for mistakes.
AtomicReference<ChangeKind> change = new AtomicReference<>();

try (DirectoryWatcher dw = new DirectoryWatcherBuilder().path(dir).listener((p, c) -> {
System.out.println("c = " + c);

@gemini-code-assist gemini-code-assist Bot Jan 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

This System.out.println appears to be a debugging statement. It should be removed to keep the test output clean.

Copilot AI Jan 3, 2026

Copy link

Choose a reason for hiding this comment

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

Debug output should be removed or replaced with proper logging. The test already has proper assertion mechanisms, and printing to System.out is not appropriate for production test code.

Suggested change
System.out.println("c = " + c);

Copilot uses AI. Check for mistakes.
change.set(c);
}).exceptionHandler(assertableExceptionHandler).build()) {
// We want it to call the listener once for setup, even without any change

Copilot AI Jan 3, 2026

Copy link

Choose a reason for hiding this comment

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

This comment is misleading. The DirectoryWatcherBuilder does not call the listener during setup by default. The existingFiles flag is not set to true in this test, so the listener will only be called when file system events occur. This comment should be corrected or removed.

Suggested change
// We want it to call the listener once for setup, even without any change
// Verify that overwriting an existing file triggers a CREATED event via the listener

Copilot uses AI. Check for mistakes.
assertableExceptionHandler.assertNoErrorInTheBackgroundThread();
java.nio.file.Files.move(file.toPath(), to.toPath(), REPLACE_EXISTING);

// Files.move(file, to);

@gemini-code-assist gemini-code-assist Bot Jan 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

This commented-out code should be removed to improve code clarity.

Comment on lines +247 to +248

Copilot AI Jan 3, 2026

Copy link

Choose a reason for hiding this comment

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

Commented-out code should be removed. If this was an alternative implementation being considered, it should either be implemented properly or removed entirely.

Suggested change
// Files.move(file, to);

Copilot uses AI. Check for mistakes.
await().atMost(5, SECONDS).until(change::get, anyOf(is(ChangeKind.CREATED), is(ChangeKind.MODIFIED)));
}
}

@Test
public void testFileWatcherWithSelectedChangeKinds() throws Throwable {
var assertableExceptionHandler = new AssertableExceptionHandler();
Expand Down Expand Up @@ -283,4 +309,4 @@ public void testFileWatcherRelativePathNPE() throws Throwable {
public void testDirectoryWatcherRelativePathNPE() throws Throwable {
new DirectoryWatcherBuilder().path(FileSystems.getDefault().getPath(".")).listener((path, changeKind) -> {}).build().close();
}
}
}
Loading