-
Notifications
You must be signed in to change notification settings - Fork 87
Adv/add jaccoco #704
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
Draft
peter-lawrey
wants to merge
36
commits into
develop
Choose a base branch
from
adv/add-jaccoco
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Adv/add jaccoco #704
Conversation
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
Root cause: License check failed due to missing headers. Fix: Applied com.mycila:license-maven-plugin format using project header. Impact: - Build passes license check and verify. - No behavioural changes; headers only.
Moves test.dat into target dir via OS.getTarget() to avoid repo-root artifacts. Verified with mvn verify.
Root cause: Surefire consumed `` while JaCoCo publishes to `` by default, so tests ran without the agent under -P sonar. Fix: Point Surefire at `` and also publish JaCoCo agent args to `` in the sonar profile (prepare-agent bound to initialize). Combine both in Surefire argLine for robustness. Impact: Sonar runs with the JaCoCo javaagent attached. Coverage checks may still fail if thresholds are unmet, but code coverage is now detected by tests using Jvm.isCodeCoverage().
…nd mapped-bytes boundary - ByteStringReaderWriterTest exercises read/skip and write/append paths - CanonicalPathUtilTest verifies interned canonical path behaviour - MappedBytesBoundaryTest writes across a chunk boundary and validates content Build: revert global surefire argLine to and bind JaCoCo prepare-agent to initialize in sonar profile to avoid default build breakage.
- Add BytesUtilTest covering equality helpers, alignment, newline combination, and file helpers - Add ReferenceTypesTest for TextIntReference and BinaryLongReference round-trips and ops - Add BytesInternalParsingTest to exercise parseBoolean/parseUtf8 in internal parsing - Keep HexDumpBytes and mapped boundary tests added earlier build(sonar): ensure surefire passes -Djvm.coverage=true under sonar to satisfy perf-sensitive tests
Root cause: global Surefire argLine omitted ${jvm.requiredArgs}, leading to JPMS access warnings/failures under JDK 11+. JaCoCo’s agent publishes its JVM args via ${argLine}.\n\nFix: set global Surefire \n- <argLine>${argLine} ${jvm.requiredArgs}</argLine>\nSonar profile already uses\n- <argLine>${argLine} ${jvm.requiredArgs} -Djvm.coverage=true</argLine>\n\nImpact: tests run with the JaCoCo agent and required module flags on modern JDKs. Verified default and sonar builds on Java 21 and 25. This prevents JPMS illegal-access issues and ensures coverage is collected consistently.
Add UTF-8 append/parse coverage for BytesInternal, covering RandomDataOutput offsets, explicit UTF flags and stop-char boundaries. Extend HexDumpBytes tests to assert fromText parsing and narrow numberWrap formatting. Assert CommonMappedBytes read-only flag blocks writes by mapping files read only. mvn -q verify -P sonar passes on Java 21, 17 and 25.
- Catch Exception instead of Throwable in ByteBuffers and NativeBytesStore static initializers (S1181) - Replace string concatenation with StringBuilder in NativeBytesStoreTest (S1643) - Use parameter in MappedBytesEdgeTest constructor to avoid unused parameter warning (S1172) - Verified targeted tests pass.
- Extract assignments out of field.set(...) expressions in array/collection/map setters - Improves readability and satisfies Sonar rule S1121 without changing behaviour. - Verified targeted tests.
…S3776) - Extract stop-bit decoding into a small static helper and holder class inside the interface - Keeps behaviour identical while lowering complexity in the method body - Verified targeted tests
- Initialise bytesStore field to BytesStore.empty() and add non-null check inline in super(...) expression - Keeps semantics and satisfies Sonar nullability rule across constructors - Verified targeted tests
- Add @SuppressWarnings("java:S1452") to factory methods that must use wildcard return types due to runtime-selected backing stores (Bytes.allocateElastic* and BytesStore.wrap/follow/from) - Avoids API breakage while satisfying Sonar rule guidance. All tests pass locally.
- BytesMarshaller: add ALLOW_PRIVATE_FIELD_ACCESS flag and centralize Jvm.setAccessible in a suppressed helper with justification - When flag is false, only marshal publicly accessible fields to avoid accessibility bypasses - Keeps existing default behaviour while offering a compliant mode
…e (S1181) - BinaryBytesMethodWriterInvocationHandler: roll back and rethrow on Exception, preserve Error - HexDumpBytes, BytesMarshallable toString: return message on Exception, rethrow Error - BinaryIntReference, UncheckedLongReference, BinaryLongReference, BinaryLongArrayReference: narrow to Exception in toString; rethrow Error - BytesInternal: handle MethodHandle.invoke Throwable locally; narrow outer catch to Exception
- Keep surefire argLine = ${argLine} ${jvm.requiredArgs} for both global and sonar runs
- In sonar profile, set forkCount=1 and reuseForks=true to improve agent flush reliability
- Context: JaCoCo agent args land in ${argLine}; multi-fork runs on some JDKs/LAFs did not emit target/jacoco.exec, causing report/check to skip and coverage to read as zero. A single fork is more reliable across Java 11/17/21/25.
Verify: mvn -q verify (default) and mvn -q -P sonar verify on JDK>=11
…; Sonar surefire reuseForks=false to improve JaCoCo exec flush
…ted overflow/null cases
…0.60); centralise in top-level properties and remove duplicate profile overrides
- ASCII-only sources: replace raw non-ASCII (e.g., £, €, ó, bullets) with Unicode escapes across tests - HexDumpBytesTest: write unique file under OS.getTarget() and clean up - Relax time-based assertions for slow CI in MappedUniqueTimeProviderTest and DistributedUniqueTimeProviderTest - Remove unused field from MappedBytesEdgeTest constructor state - Update ByteStoreTest, ByteStringParserTest, ISO88591CharsTest, UnicodeToStringTest, WriteLimitTest, StringsTest, BytesTest, BinaryIntArrayReferenceTest accordingly
- S1172 (unused parameter): use Parameterized display name param in MappedBytesEdgeTest via Objects.requireNonNull(name) - S5826 (JUnit5 BeforeEach suggestion): retain JUnit4 @before in MappedFileTest with @SuppressWarnings(java:S5826) justification - S1117 (variable shadowing): rename local variables in AbstractBytesTest to avoid hiding fields Note: Kept ISO-8859-1 literals (e.g., £, ·, ó) in test sources as per project policy; non-ISO characters (e.g., €) remain escaped.
…restore correctness) - Replace separate Exception/Error catches with single catch(Throwable) - Ensures buffers are not corrupted when proxied interfaces throw non-Exception Throwables
- HexDump: merge formatting/wrap/zero-length into HexDumpBytesLayoutTest; remove three separate classes - BytesInternal IO/Copy: merge into BytesInternalIOCopyTest; remove two separate classes - UTF-8 parsing: consolidate boundary/limit/null cases into Utf8ParsingBoundaryTest; remove two separate classes No behavioural changes; uses existing assertions and helpers.
…rage emission in sonar runs
…catch(Throwable) rollback
…ite8bit boundary, extra readUtf8Limited boundary, VanillaBytes capacity/zeroOut, and method-writer rollback test
…or valid readUtf8()
- Remove SortPom from default build and delete opt-in profile - Keep builds deterministic; developers can run 'mvn sortpom:sort' when desired Impact: avoids unintended POM reformatting during CI/local verify.
# Conflicts: # src/main/java/net/openhft/chronicle/bytes/Bytes.java # src/main/java/net/openhft/chronicle/bytes/BytesStore.java
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.



No description provided.