-
Notifications
You must be signed in to change notification settings - Fork 5.9k
refactor(tests): Eliminate test flakiness through deterministic implementation #4708
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.
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
base: main
Are you sure you want to change the base?
refactor(tests): Eliminate test flakiness through deterministic implementation #4708
Conversation
…ed values and improve test stability by removing `Thread.sleep` with busy-wait loops and consistent timestamps. Signed-off-by: Balázs Szücs <bszucs1209@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Refactors tests to reduce flakiness by eliminating randomness and system-time dependencies, standardizing string identifiers, and clarifying time-based logic. Also disables a particularly flaky test and adjusts a timeout simulation approach.
- Replace random UUIDs with deterministic string IDs in tests
- Swap time-dependent calls with fixed or clearer time values and comments
- Disable a flaky recursive cleanup test; change a timeout simulation from sleep to a busy-wait loop
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| app/core/src/test/java/stirling/software/SPDF/pdf/TextFinderTest.java | Use fixed start/end timestamps to avoid timing flakiness. |
| app/common/src/test/java/stirling/software/common/util/FileMonitorTest.java | Set deterministic lastModifiedTime values for test files/dirs. |
| app/common/src/test/java/stirling/software/common/service/TempFileCleanupServiceTest.java | Clarify comments, NPE-safe string comparisons, fixed timestamps in one test, and disable a flaky test. |
| app/common/src/test/java/stirling/software/common/service/TaskManagerTest.java | Replace UUIDs with fixed job IDs. |
| app/common/src/test/java/stirling/software/common/service/ResourceMonitorTest.java | Clarify relative time via comment. |
| app/common/src/test/java/stirling/software/common/service/JobExecutorServiceTest.java | Replace Thread.sleep with a busy-wait to simulate long-running work. |
| app/common/src/test/java/stirling/software/common/service/FileStorageTest.java | Replace UUIDs with fixed file IDs. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
app/common/src/test/java/stirling/software/common/service/TempFileCleanupServiceTest.java
Outdated
Show resolved
Hide resolved
app/common/src/test/java/stirling/software/common/service/TempFileCleanupServiceTest.java
Outdated
Show resolved
Hide resolved
app/common/src/test/java/stirling/software/common/service/JobExecutorServiceTest.java
Show resolved
Hide resolved
app/common/src/test/java/stirling/software/common/service/TempFileCleanupServiceTest.java
Outdated
Show resolved
Hide resolved
…FileCleanupServiceTest.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Balázs Szücs <bszucs1209@gmail.com>
… by enhancing file timestamp mocking and removing debug prints. Signed-off-by: Balázs Szücs <bszucs1209@gmail.com>
…meMillis` and `Instant.now` with deterministic test timestamps. Signed-off-by: Balázs Szücs <bszucs1209@gmail.com>
# Conflicts: # app/common/src/test/java/stirling/software/common/model/FileInfoTest.java # app/common/src/test/java/stirling/software/common/service/TaskManagerTest.java # app/common/src/test/java/stirling/software/common/service/TempFileCleanupServiceTest.java
- Changed private constants `TEST_MOD_DATE` and `TEST_CREATION_DATE` to package-private for reuse. - Reformatted nested `DateFormattingTests` for consistent indentation. - Updated test assertions to use prefixed constants for better clarity. - Removed redundant closing bracket. Signed-off-by: Balázs Szücs <bszucs1209@gmail.com>
- Replaced test timestamps with a fixed `FIXED_NOW` constant for deterministic testing. - Updated test assertions to use adjusted timestamps for modifications and creations (e.g., `FIXED_NOW`, `FIXED_NOW.minusDays(1)`). - Simplified file path construction using static strings instead of `File.separator`. - Added test cases for null handling in modification and creation dates. - Added tests for edge cases in file size formatting (e.g., rounding for KB, large sizes in GB). Signed-off-by: Balázs Szücs <bszucs1209@gmail.com>
Description of Changes
Updated test files to use fixed string identifiers and timestamps instead of random UUIDs and system-dependent times. These changes make the tests more deterministic and easier to debug.
Test determinism and clarity improvements:
FileStorageTest.javaandTaskManagerTest.javato ensure predictable test data.System.currentTimeMillis()andInstant.now()to fixed values in tests forTempFileCleanupServiceTest.java,FileMonitorTest.java, andTextFinderTest.javato avoid flakiness due to timing issues.equals()where appropriate.JobExecutorServiceTest.javato use busy-waiting instead ofThread.sleep, reducing test flakiness and improving reliability.Checklist
General
Documentation
UI Changes (if applicable)
Testing (if applicable)