[fix](regression) Wait past unsafe time boundaries#65742
Conversation
### What problem does this PR solve? Issue Number: None Related PR: apache#65714 Problem Summary: The regression framework truncated the remaining time before an hour or day boundary to whole seconds. Sleeping for that truncated value could resume in the final fractional second before the boundary, allowing a boundary-sensitive case to cross it and fail intermittently. Compute the wait in milliseconds and sleep one additional millisecond so execution resumes strictly after the boundary. Add focused coverage for fractional, exact, and safe boundary windows, and align the JUnit API with the engine already provided by Groovy. ### Release note None ### Check List (For Author) - Test: Unit Test - mvn -o -f regression-test/framework/pom.xml test - Behavior changed: Yes. Boundary-sensitive regression cases now resume strictly after an unsafe hour or day boundary. - Does this need documentation: No
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
/review |
There was a problem hiding this comment.
Review result: changes requested for one test-integration gap.
Critical checkpoint conclusions:
- Goal and proof: The millisecond calculation fixes the reported whole-second truncation under the existing stable-clock contract. Fractional, exact-boundary, and just-safe helper cases are asserted, but the checked-in standard framework pipeline suppresses this test class, so the regression proof is not enforced by automation.
- Scope and clarity: The production change is small and focused; the JUnit API bump aligns with Groovy 4.0.19's Jupiter engine/launcher dependency set.
- Concurrency and lifecycle: No shared mutable state, lock ordering, or special lifecycle is introduced. The suite worker's blocking sleep is intentional. Adjustable-clock behavior remains a pre-existing limitation of the same one-shot LocalDateTime/Thread.sleep design and is not worsened here.
- Configuration, compatibility, persistence, and parallel paths: No configuration, storage/protocol, transaction, persistence, FE/BE transport, or rolling-upgrade concern applies. Both hour/day branches and all three current callers were traced; they share the corrected helper and use durations below their boundary periods.
- Testing and results: The new fixed-time JUnit cases are deterministic and no generated regression
.outapplies. Maven can compile and discover them with aligned Jupiter components, butrun-regression-test.sh --compilepackages with-DskipTests=true, and no tracked pipeline/workflow runs the framework test phase. This must be wired into a preset automated path. - Observability and performance: Existing wait logging remains sufficient; the change adds one calculation and the intentional unsafe-window sleep, with no hot-path or allocation concern.
- Additional user focus: No additional focus was provided.
Validation was static only as required by this review runner; no builds or tests were executed.
| import static org.junit.jupiter.api.Assertions.assertTrue | ||
|
|
||
| class SuiteBoundaryWaitTest { | ||
| @Test |
There was a problem hiding this comment.
[P1] Run these tests in the checked-in pipeline
The standard framework path never executes this class: run-regression-test.sh --compile reaches mvn clean package ... -DskipTests=true, and regression-test/pipeline/common/doris-utils.sh prepares the framework only through that entry point. I could not find any tracked workflow or pipeline command that invokes the framework's Maven test phase, so these methods compile but cannot fail CI and the boundary regression remains unprotected. Please wire the framework unit-test phase into a preset/automated path and verify all three tests appear in the Surefire results.
### What problem does this PR solve? Issue Number: None Related PR: apache#65714 Problem Summary: The standard regression framework compile preset packaged the framework with tests skipped, so the framework unit tests added for boundary waiting were absent from the checked-in pipeline path. Run the framework test phase during the standard package command while leaving the separate Java UDF build unchanged. ### Release note None ### Check List (For Author) - Test: Unit Test - ./run-regression-test.sh --clean --compile - Behavior changed: Yes. The standard regression framework compile preset now executes framework unit tests. - Does this need documentation: No
|
run buildall |
|
/review |
There was a problem hiding this comment.
Follow-up review result for 6d0d524c765def30e21a2528a3e20412f37730c1: no new blocking code issue found. The previous P1 is resolved on the checked-in automated path.
Critical checkpoint conclusions:
- Goal and proof: The change fixes the whole-second truncation by computing the positive boundary delta in milliseconds and sleeping for its floor plus one, which resumes strictly after the sampled hour/day boundary. The deterministic helper tests cover fractional, exact-duration, and just-safe cases.
- Scope and clarity: The production change is small and focused. The JUnit API update aligns with Groovy 4.0.19's Jupiter engine and Platform launcher.
- Concurrency and lifecycle: No shared mutable state, lock ordering, static-initialization dependency, or special resource lifecycle is introduced. The intentional blocking suite-worker sleep remains local.
- Configuration, compatibility, persistence, and parallel paths: No configuration, storage/protocol, transaction, persistence, FE/BE transport, or rolling-upgrade concern applies. Both hour/day branches and all three current callers use the shared corrected helper.
- Testing and results: GMavenPlus compiles the Groovy test source and Surefire discovers
SuiteBoundaryWaitTest. Removing-DskipTests=truefrom the clean package command makes the standard--clean/--compilepipeline execute and enforce the three methods, resolving the existing P1. No generated regression.outapplies. - Observability and performance: Existing INFO logging reports the intentional wait in milliseconds. The arithmetic/test overhead is negligible; no hot-path allocation or repeated-work issue was found.
- Additional user focus: No additional focus was provided.
Validation was static only as required by this review runner; no build or test command was executed. The live external COMPILE check is currently failing, but its TeamCity log requires authentication and no cause can be attributed from the available code evidence, so this review does not claim that CI is green.
What problem does this PR solve?
Issue Number: None
Related PR: #65714
Problem Summary: The regression framework truncated the remaining time before an hour or day boundary to whole seconds. Sleeping for that truncated value could resume in the final fractional second before the boundary, allowing a boundary-sensitive case to cross it and fail intermittently. This change computes the wait in milliseconds and sleeps one additional millisecond so execution resumes strictly after the boundary. It also adds focused coverage for fractional, exact, and safe boundary windows and aligns the JUnit API with the engine already provided by Groovy.
Release note
None
Check List (For Author)
Test
mvn -o -f regression-test/framework/pom.xml test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)