Use Locale.ROOT with String.format which does or could render numbers - #2320
Merged
Conversation
Member
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR standardizes string formatting to use Locale.ROOT across many String.format/printf call sites (compiler, runtime, utils, concurrency, and Spring integration); no public APIs or control-flow semantics were changed. ChangesCompiler / Spec rewriting
Mocking / Interaction
Iteration & Naming Providers
Comparison Renderers
Exceptions & Runtime
Utilities & Model
Timeouts & Extensions
Concurrency utils
Snapshot / Special cases
Spring mock integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR systematically adds Key aspects of the change:
Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["String.format call site"] --> B{Format string\ncontains numbers?}
B -- "Yes (%d, %f, %e, etc.)" --> C["Use String.format(Locale.ROOT, ...)"]
B -- "No (only %s, %b, %c)" --> D["Use String.format(...) — no locale needed"]
C --> E[Consistent output\nregardless of JVM locale]
D --> F[Locale-independent\nstring substitution]
E --> G["Machine messages, error text,\nversion strings, debug output"]
F --> G
style C fill:#2d8a4e,color:#fff
style D fill:#2d6ca8,color:#fff
style E fill:#1a5c33,color:#fff
style F fill:#1a4a7a,color:#fff
Last reviewed commit: f03bda4 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
spock-core/src/main/java/org/spockframework/runtime/extension/builtin/TimeoutInterceptor.java (2)
136-142: Inconsistent locale handling withprintfcalls.This method formats numbers with
%1.2fbut doesn't useLocale.ROOT, unlike the updatedString.formatcall at line 126. For consistency with the PR's objective, consider updating theseprintfcalls.♻️ Suggested fix for consistency
private void logUnsuccessfulInterrupt(String methodName, long now, long timeoutAt, long waitMillis, int unsuccessfulAttempts) { - System.err.printf( + System.err.printf(Locale.ROOT, "[spock.lang.Timeout] Method '%s' has not stopped after timing out %1.2f seconds ago - interrupting. Next try in %1.2f seconds.\n%n", methodName, TimeUtil.toSeconds(Duration.ofNanos(now - timeoutAt)), waitMillis / 1000d );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spock-core/src/main/java/org/spockframework/runtime/extension/builtin/TimeoutInterceptor.java` around lines 136 - 142, The System.err.printf call in logUnsuccessfulInterrupt formats floats without a fixed locale; update the call to use Locale.ROOT (similar to the String.format change) so numeric formatting is consistent across locales—locate the logUnsuccessfulInterrupt method and replace the System.err.printf invocation with a variant that supplies Locale.ROOT while preserving the same format string and argument order.
154-160: Inconsistent locale handling withprintfcall.Same issue as
logUnsuccessfulInterrupt- this method formats%1.2fwithoutLocale.ROOT.♻️ Suggested fix for consistency
private void logMethodTimeout(String methodName, double timeoutSeconds) { - System.err.printf( + System.err.printf(Locale.ROOT, "[spock.lang.Timeout] Method '%s' timed out after %1.2f seconds.%n", methodName, timeoutSeconds );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spock-core/src/main/java/org/spockframework/runtime/extension/builtin/TimeoutInterceptor.java` around lines 154 - 160, logMethodTimeout uses System.err.printf with a floating-point format without specifying Locale, causing locale-dependent formatting; update logMethodTimeout to call System.err.printf with Locale.ROOT as the first argument (mirroring logUnsuccessfulInterrupt) so the %1.2f is always formatted consistently, and ensure java.util.Locale is available/imported if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@spock-core/src/main/java/spock/util/concurrent/PollingConditions.java`:
- Around line 201-203: The code mixes locale-sensitive numeric formatting: some
numeric formats use Locale.ROOT (e.g., the String.format call that formats
elapsedTime/attempts) while other printf/String.format usages still rely on the
default locale when formatting "%1.2f". Locate the printf/String.format calls
that format numbers with "%1.2f" (the ones that reference
elapsedTime/attempts-style values) and change them to use Locale.ROOT (e.g.,
String.format(Locale.ROOT, ...) or printf(Locale.ROOT, ...)) so all numeric
formatting in PollingConditions uses Locale.ROOT consistently.
---
Nitpick comments:
In
`@spock-core/src/main/java/org/spockframework/runtime/extension/builtin/TimeoutInterceptor.java`:
- Around line 136-142: The System.err.printf call in logUnsuccessfulInterrupt
formats floats without a fixed locale; update the call to use Locale.ROOT
(similar to the String.format change) so numeric formatting is consistent across
locales—locate the logUnsuccessfulInterrupt method and replace the
System.err.printf invocation with a variant that supplies Locale.ROOT while
preserving the same format string and argument order.
- Around line 154-160: logMethodTimeout uses System.err.printf with a
floating-point format without specifying Locale, causing locale-dependent
formatting; update logMethodTimeout to call System.err.printf with Locale.ROOT
as the first argument (mirroring logUnsuccessfulInterrupt) so the %1.2f is
always formatted consistently, and ensure java.util.Locale is available/imported
if needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ec1fca92-9b83-4e75-8810-2aa66d783fb0
📒 Files selected for processing (30)
spock-core/src/main/java/org/spockframework/compiler/ErrorReporter.javaspock-core/src/main/java/org/spockframework/compiler/InvalidSpecCompileException.javaspock-core/src/main/java/org/spockframework/compiler/SpecRewriter.javaspock-core/src/main/java/org/spockframework/compiler/WhereBlockRewriter.javaspock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMockFactory.javaspock-core/src/main/java/org/spockframework/mock/runtime/MockInteraction.javaspock-core/src/main/java/org/spockframework/runtime/DataVariablesIterationNameProvider.javaspock-core/src/main/java/org/spockframework/runtime/FailedSetEqualityComparisonRenderer.javaspock-core/src/main/java/org/spockframework/runtime/FailedStringComparisonRenderer.javaspock-core/src/main/java/org/spockframework/runtime/InvalidSpecException.javaspock-core/src/main/java/org/spockframework/runtime/SafeIterationNameProvider.javaspock-core/src/main/java/org/spockframework/runtime/SpockExecutionException.javaspock-core/src/main/java/org/spockframework/runtime/SpockRuntime.javaspock-core/src/main/java/org/spockframework/runtime/extension/ExtensionException.javaspock-core/src/main/java/org/spockframework/runtime/extension/builtin/TimeoutInterceptor.javaspock-core/src/main/java/org/spockframework/runtime/model/TextPosition.javaspock-core/src/main/java/org/spockframework/util/Assert.javaspock-core/src/main/java/org/spockframework/util/InternalSpockError.javaspock-core/src/main/java/org/spockframework/util/IoUtil.javaspock-core/src/main/java/org/spockframework/util/VersionNumber.javaspock-core/src/main/java/spock/config/ConfigurationException.javaspock-core/src/main/java/spock/lang/Snapshotter.javaspock-core/src/main/java/spock/mock/AutoAttachExtension.javaspock-core/src/main/java/spock/util/concurrent/AsyncConditions.javaspock-core/src/main/java/spock/util/concurrent/BlockingVariable.javaspock-core/src/main/java/spock/util/concurrent/PollingConditions.javaspock-spring/src/main/java/org/spockframework/spring/mock/DelegatingInterceptor.javaspock-spring/src/main/java/org/spockframework/spring/mock/SpockDefinition.javaspock-spring/src/main/java/org/spockframework/spring/mock/SpockMockPostprocessor.javaspock-spring/src/main/java/org/spockframework/spring/mock/SpyDefinition.java
Vampire
force-pushed
the
vampire/fix-number-rendering
branch
from
March 13, 2026 16:00
f03bda4 to
50f48e6
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@spock-core/src/main/java/org/spockframework/runtime/extension/builtin/TimeoutInterceptor.java`:
- Line 139: In TimeoutInterceptor.java (class TimeoutInterceptor), the format
string used when logging the timeout message includes both a literal "\n" and
"%n", causing double newlines; update the format string in the logging/format
call (the string starting with "[spock.lang.Timeout] Method '%s'...") to use a
single platform-specific line separator (prefer %n) and remove the redundant
"\n" so the message ends with only "%n".
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3807fb9a-fe8a-4524-b5b7-e599d74fea64
📒 Files selected for processing (30)
spock-core/src/main/java/org/spockframework/compiler/ErrorReporter.javaspock-core/src/main/java/org/spockframework/compiler/InvalidSpecCompileException.javaspock-core/src/main/java/org/spockframework/compiler/SpecRewriter.javaspock-core/src/main/java/org/spockframework/compiler/WhereBlockRewriter.javaspock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMockFactory.javaspock-core/src/main/java/org/spockframework/mock/runtime/MockInteraction.javaspock-core/src/main/java/org/spockframework/runtime/DataVariablesIterationNameProvider.javaspock-core/src/main/java/org/spockframework/runtime/FailedSetEqualityComparisonRenderer.javaspock-core/src/main/java/org/spockframework/runtime/FailedStringComparisonRenderer.javaspock-core/src/main/java/org/spockframework/runtime/InvalidSpecException.javaspock-core/src/main/java/org/spockframework/runtime/SafeIterationNameProvider.javaspock-core/src/main/java/org/spockframework/runtime/SpockExecutionException.javaspock-core/src/main/java/org/spockframework/runtime/SpockRuntime.javaspock-core/src/main/java/org/spockframework/runtime/extension/ExtensionException.javaspock-core/src/main/java/org/spockframework/runtime/extension/builtin/TimeoutInterceptor.javaspock-core/src/main/java/org/spockframework/runtime/model/TextPosition.javaspock-core/src/main/java/org/spockframework/util/Assert.javaspock-core/src/main/java/org/spockframework/util/InternalSpockError.javaspock-core/src/main/java/org/spockframework/util/IoUtil.javaspock-core/src/main/java/org/spockframework/util/VersionNumber.javaspock-core/src/main/java/spock/config/ConfigurationException.javaspock-core/src/main/java/spock/lang/Snapshotter.javaspock-core/src/main/java/spock/mock/AutoAttachExtension.javaspock-core/src/main/java/spock/util/concurrent/AsyncConditions.javaspock-core/src/main/java/spock/util/concurrent/BlockingVariable.javaspock-core/src/main/java/spock/util/concurrent/PollingConditions.javaspock-spring/src/main/java/org/spockframework/spring/mock/DelegatingInterceptor.javaspock-spring/src/main/java/org/spockframework/spring/mock/SpockDefinition.javaspock-spring/src/main/java/org/spockframework/spring/mock/SpockMockPostprocessor.javaspock-spring/src/main/java/org/spockframework/spring/mock/SpyDefinition.java
🚧 Files skipped from review as they are similar to previous changes (18)
- spock-core/src/main/java/org/spockframework/runtime/SpockRuntime.java
- spock-core/src/main/java/spock/lang/Snapshotter.java
- spock-core/src/main/java/org/spockframework/runtime/model/TextPosition.java
- spock-core/src/main/java/org/spockframework/util/VersionNumber.java
- spock-core/src/main/java/org/spockframework/runtime/DataVariablesIterationNameProvider.java
- spock-core/src/main/java/org/spockframework/compiler/SpecRewriter.java
- spock-spring/src/main/java/org/spockframework/spring/mock/SpockMockPostprocessor.java
- spock-core/src/main/java/org/spockframework/runtime/SafeIterationNameProvider.java
- spock-core/src/main/java/spock/config/ConfigurationException.java
- spock-core/src/main/java/org/spockframework/runtime/FailedStringComparisonRenderer.java
- spock-spring/src/main/java/org/spockframework/spring/mock/SpockDefinition.java
- spock-core/src/main/java/spock/util/concurrent/AsyncConditions.java
- spock-core/src/main/java/org/spockframework/runtime/FailedSetEqualityComparisonRenderer.java
- spock-core/src/main/java/org/spockframework/compiler/InvalidSpecCompileException.java
- spock-core/src/main/java/org/spockframework/util/Assert.java
- spock-core/src/main/java/org/spockframework/compiler/ErrorReporter.java
- spock-spring/src/main/java/org/spockframework/spring/mock/DelegatingInterceptor.java
- spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMockFactory.java
| private void logUnsuccessfulInterrupt(String methodName, long now, long timeoutAt, long waitMillis, int unsuccessfulAttempts) { | ||
| System.err.printf( | ||
| Locale.ROOT, | ||
| "[spock.lang.Timeout] Method '%s' has not stopped after timing out %1.2f seconds ago - interrupting. Next try in %1.2f seconds.\n%n", |
There was a problem hiding this comment.
Minor: Redundant newline characters.
The format string has both \n (literal newline) and %n (platform-specific line separator) at the end, resulting in double newlines in the output.
🔧 Suggested fix
- "[spock.lang.Timeout] Method '%s' has not stopped after timing out %1.2f seconds ago - interrupting. Next try in %1.2f seconds.\n%n",
+ "[spock.lang.Timeout] Method '%s' has not stopped after timing out %1.2f seconds ago - interrupting. Next try in %1.2f seconds.%n",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "[spock.lang.Timeout] Method '%s' has not stopped after timing out %1.2f seconds ago - interrupting. Next try in %1.2f seconds.\n%n", | |
| "[spock.lang.Timeout] Method '%s' has not stopped after timing out %1.2f seconds ago - interrupting. Next try in %1.2f seconds.%n", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@spock-core/src/main/java/org/spockframework/runtime/extension/builtin/TimeoutInterceptor.java`
at line 139, In TimeoutInterceptor.java (class TimeoutInterceptor), the format
string used when logging the timeout message includes both a literal "\n" and
"%n", causing double newlines; update the format string in the logging/format
call (the string starting with "[spock.lang.Timeout] Method '%s'...") to use a
single platform-specific line separator (prefer %n) and remove the redundant
"\n" so the message ends with only "%n".
Vampire
force-pushed
the
vampire/fix-number-rendering
branch
from
March 19, 2026 16:28
50f48e6 to
d166ea1
Compare
This comment has been minimized.
This comment has been minimized.
Vampire
force-pushed
the
vampire/fix-number-rendering
branch
from
March 21, 2026 21:35
d166ea1 to
ccbd6f6
Compare
Vampire
force-pushed
the
vampire/fix-number-rendering
branch
from
May 28, 2026 13:35
ccbd6f6 to
138d408
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
spock-core/src/main/java/org/spockframework/runtime/extension/builtin/TimeoutInterceptor.java (1)
136-143:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRedundant newline characters.
The format string at line 139 ends with both
\n(literal newline) and%n(platform-specific line separator), resulting in double newlines in the output.🔧 Suggested fix
- "[spock.lang.Timeout] Method '%s' has not stopped after timing out %1.2f seconds ago - interrupting. Next try in %1.2f seconds.\n%n", + "[spock.lang.Timeout] Method '%s' has not stopped after timing out %1.2f seconds ago - interrupting. Next try in %1.2f seconds.%n",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@spock-core/src/main/java/org/spockframework/runtime/extension/builtin/TimeoutInterceptor.java` around lines 136 - 143, In logUnsuccessfulInterrupt, the format string passed to System.err.printf in method logUnsuccessfulInterrupt contains both a literal "\n" and "%n" causing double newlines; update the format string (in the System.err.printf call in logUnsuccessfulInterrupt) to use only one newline marker (prefer using "%n" and remove the "\n") so the message prints a single line break.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In
`@spock-core/src/main/java/org/spockframework/runtime/extension/builtin/TimeoutInterceptor.java`:
- Around line 136-143: In logUnsuccessfulInterrupt, the format string passed to
System.err.printf in method logUnsuccessfulInterrupt contains both a literal
"\n" and "%n" causing double newlines; update the format string (in the
System.err.printf call in logUnsuccessfulInterrupt) to use only one newline
marker (prefer using "%n" and remove the "\n") so the message prints a single
line break.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f80755f4-27e5-4a21-8036-da043b22f542
📒 Files selected for processing (30)
spock-core/src/main/java/org/spockframework/compiler/ErrorReporter.javaspock-core/src/main/java/org/spockframework/compiler/InvalidSpecCompileException.javaspock-core/src/main/java/org/spockframework/compiler/SpecRewriter.javaspock-core/src/main/java/org/spockframework/compiler/WhereBlockRewriter.javaspock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMockFactory.javaspock-core/src/main/java/org/spockframework/mock/runtime/MockInteraction.javaspock-core/src/main/java/org/spockframework/runtime/DataVariablesIterationNameProvider.javaspock-core/src/main/java/org/spockframework/runtime/FailedSetEqualityComparisonRenderer.javaspock-core/src/main/java/org/spockframework/runtime/FailedStringComparisonRenderer.javaspock-core/src/main/java/org/spockframework/runtime/InvalidSpecException.javaspock-core/src/main/java/org/spockframework/runtime/SafeIterationNameProvider.javaspock-core/src/main/java/org/spockframework/runtime/SpockExecutionException.javaspock-core/src/main/java/org/spockframework/runtime/SpockRuntime.javaspock-core/src/main/java/org/spockframework/runtime/extension/ExtensionException.javaspock-core/src/main/java/org/spockframework/runtime/extension/builtin/TimeoutInterceptor.javaspock-core/src/main/java/org/spockframework/runtime/model/TextPosition.javaspock-core/src/main/java/org/spockframework/util/Assert.javaspock-core/src/main/java/org/spockframework/util/InternalSpockError.javaspock-core/src/main/java/org/spockframework/util/IoUtil.javaspock-core/src/main/java/org/spockframework/util/VersionNumber.javaspock-core/src/main/java/spock/config/ConfigurationException.javaspock-core/src/main/java/spock/lang/Snapshotter.javaspock-core/src/main/java/spock/mock/AutoAttachExtension.javaspock-core/src/main/java/spock/util/concurrent/AsyncConditions.javaspock-core/src/main/java/spock/util/concurrent/BlockingVariable.javaspock-core/src/main/java/spock/util/concurrent/PollingConditions.javaspock-spring/src/main/java/org/spockframework/spring/mock/DelegatingInterceptor.javaspock-spring/src/main/java/org/spockframework/spring/mock/SpockDefinition.javaspock-spring/src/main/java/org/spockframework/spring/mock/SpockMockPostprocessor.javaspock-spring/src/main/java/org/spockframework/spring/mock/SpyDefinition.java
✅ Files skipped from review due to trivial changes (10)
- spock-core/src/main/java/org/spockframework/compiler/SpecRewriter.java
- spock-core/src/main/java/spock/util/concurrent/AsyncConditions.java
- spock-core/src/main/java/org/spockframework/util/InternalSpockError.java
- spock-core/src/main/java/org/spockframework/util/VersionNumber.java
- spock-core/src/main/java/spock/mock/AutoAttachExtension.java
- spock-spring/src/main/java/org/spockframework/spring/mock/DelegatingInterceptor.java
- spock-core/src/main/java/org/spockframework/compiler/WhereBlockRewriter.java
- spock-spring/src/main/java/org/spockframework/spring/mock/SpockMockPostprocessor.java
- spock-core/src/main/java/org/spockframework/runtime/FailedStringComparisonRenderer.java
- spock-core/src/main/java/org/spockframework/util/IoUtil.java
🚧 Files skipped from review as they are similar to previous changes (16)
- spock-core/src/main/java/spock/util/concurrent/PollingConditions.java
- spock-spring/src/main/java/org/spockframework/spring/mock/SpyDefinition.java
- spock-core/src/main/java/org/spockframework/runtime/SpockExecutionException.java
- spock-core/src/main/java/org/spockframework/runtime/InvalidSpecException.java
- spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMockFactory.java
- spock-core/src/main/java/org/spockframework/runtime/SpockRuntime.java
- spock-core/src/main/java/spock/config/ConfigurationException.java
- spock-core/src/main/java/org/spockframework/runtime/model/TextPosition.java
- spock-core/src/main/java/org/spockframework/util/Assert.java
- spock-spring/src/main/java/org/spockframework/spring/mock/SpockDefinition.java
- spock-core/src/main/java/org/spockframework/mock/runtime/MockInteraction.java
- spock-core/src/main/java/org/spockframework/compiler/InvalidSpecCompileException.java
- spock-core/src/main/java/org/spockframework/runtime/extension/ExtensionException.java
- spock-core/src/main/java/spock/lang/Snapshotter.java
- spock-core/src/main/java/spock/util/concurrent/BlockingVariable.java
- spock-core/src/main/java/org/spockframework/runtime/DataVariablesIterationNameProvider.java
✅ All tests passed ✅🏷️ Commit: 138d408 Learn more about TestLens at testlens.app. |
leonard84
approved these changes
May 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit