Chore: migrate tests from jmockit to mockito to include test coverage in sonar report#229
Open
mcook42 wants to merge 51 commits intopmonington/upgrade-springfrom
Open
Chore: migrate tests from jmockit to mockito to include test coverage in sonar report#229mcook42 wants to merge 51 commits intopmonington/upgrade-springfrom
mcook42 wants to merge 51 commits intopmonington/upgrade-springfrom
Conversation
Removed @mocked on the J2735Bsm field — this was a misuse of jmockit for a plain getter/setter test. Replaced with a real instance and migrated imports from JUnit 4 Assert to JUnit 5 Assertions.
Migrated 22 of 28 tests. Patterns used: - @tested + @Injectable + @capturing -> @Injectmocks + @mock + MockedConstruction - new Expectations() { session.set(...); result = ...; } -> mockConstruction(SnmpSession.class, (mock, ctx) -> when(mock.set(...)).thenReturn/thenThrow(...)) - Extracted helper methods (mockSnmpSession, stubSessionReturning, assertBadRequestForErrorCode) to consolidate 9 near-duplicate tests The remaining 6 tests (deleteShouldCatchSessionIOException* and deleteShouldCatchSessionNullPointerException*) are @disabled with a TODO because pure Mockito cannot simulate a constructor throwing a checked exception: MockedConstruction wraps any throwable raised from the initializer in MockitoException, which the controller's catch(IOException) and catch(NullPointerException) blocks do not match. Unblocking options: inject a SnmpSession factory into TimDeleteController or adopt PowerMockito.whenNew.
The file had no live jmockit usage — only a commented-out @RunWith(JMockit.class) and a stale org.junit.runner.RunWith import. Removed dead imports and switched org.junit.Assert.assertEquals to org.junit.jupiter.api.Assertions.assertEquals. All 3 tests pass.
Removed dead jmockit imports and JUnit 4 residue; the file had only commented-out JMockit annotations and was already using JUnit 5 semantics. Aligned assertEquals argument order to JUnit 5 (expected, actual, message).
…mockit to Mockito
Replaced the 5 @capturing field mocks with per-test MockedStatic instances managed in @BeforeEach/@AfterEach. The tests do not stub or verify on the mocked builders, so Mockito's default null-returning stubs are equivalent to jmockit's @capturing behavior here.
… jmockit to Mockito
Removed the MockUp<LoggerFactory> that stubbed logger creation; the real SLF4J logger is harmless in these tests and nothing references the mock after construction.
…om jmockit to Mockito Removed the MockUp<LoggerFactory> that stubbed logger creation; the real SLF4J logger is harmless in these tests and nothing references the mock after construction.
Migrated 2 of 3 tests. testSetChosenFieldException2 is @disabled because the original relied on jmockit replacing the static final Logger field in J2735Choice and making logger.error throw a checked NoSuchFieldException from inside a catch block; pure Mockito cannot replace a static field once the class has loaded and rejects checked-exception stubbing on Logger.error. Unblock by either refactoring J2735Choice to rethrow instead of log-and-swallow, or by adopting PowerMock.
Dropped the jmockit verification of Thread.sleep from testGreeting because Mockito cannot mock java.lang.Thread.sleep (risk of deadlocking class loading). The getName() verification still proves the greeting method body was executed.
Used MockedConstruction for DefaultUdpTransportMapping/Snmp/USM (each intercepts new X(...)) and MockedStatic for SecurityModels.getInstance and RsuSnmp static methods, all within try-with-resources blocks per test.
Used MockedConstruction<SnmpSession> with a per-test SnmpSession.set stub, and MockedStatic for JsonUtils.fromJson and PdmUtil.createPDU (the latter lets the test short-circuit PdmUtil's real PDU-assembly logic). PDU matcher widened to any() so null pdu args (from the mocked createPDU) still match.
Migrated 8 of 9 tests. constructorShouldWithIOException is @disabled because pure Mockito cannot simulate a constructor throwing a checked exception. Unblock via PowerMock or production refactor.
Migrated 18 of 21 tests. 3 snmpSessionException* tests are @disabled because pure Mockito cannot simulate SnmpSession constructor throwing IOException (see TimDeleteControllerTest for details). Consolidated 15 copy-paste-per-protocol tests behind three helpers (assertListenIoThrowsYields500, assertSendReturning, assertSuccessfulQuery) to keep the migrated form readable.
Converted both @Capturing-param tests to MockedStatic patterns. For TimTransmogrifier (which has multiple static methods the production code calls), used Mockito.CALLS_REAL_METHODS as the default so only convertToXml is replaced with a throwing stub; updateRsuCreds/buildASD/etc. run real.
All tests in jpo-ode-core/plugins/svcs have been migrated to Mockito. This removes the jmockit dependency and the -javaagent line from the surefire argLine. Spring Boot 4.0.3's starter-test still brings in Mockito 5.x with the inline mock maker, which is all the migrated tests need.
Deleted the @disabled test `constructorShouldWithIOException` in `SnmpSessionTest` as the mocking limitations remain unresolved.
…njection Replaced `SnmpSession` constructor mocking with a factory pattern by injecting `SnmpSessionFactory`. Updated `TimDeleteController` and `TimQueryController` accordingly. Adjusted related tests for simplified mocking support.
Deleted the @disabled test `testSetChosenFieldException2` in `J2735ChoiceTest` as its mocking limitations remain unresolved and would require a production refactor or PowerMock to unblock.
Adds the factory infrastructure relied on by the SnmpSession refactor in 02ac21f: - SnmpSessionFactory functional interface - TransportMappingFactory functional interface - SnmpConfig @bean wiring SnmpSession::new as the default factory These were missed when staging the refactor commit; without them HEAD will not compile.
Reinstates interaction checks lost when these tests were converted from JMockit `Expectations`/`Verifications` blocks: - RsuHealthControllerTest now verifies RsuSnmp.sendSnmpV3Request was called with the expected username decoded from the auth header (null for no auth, "aladdin" for "Basic YWxhZGRpbjpvcGVuc2VzYW1l"). Previously the tests passed without exercising the Basic-auth decoding path. - RsuSnmpTest now verifies snmp.close() on the three success/no-response paths and verify(snmp, never()).close() on the IOException path, pinning down the resource-management contract of sendSnmpV3Request. - StompControllerTest.testGreeting: replaced the long jmockit-rationale comment with a one-liner. The existing getName() verification (which runs after Thread.sleep) already covers what the dropped Thread.sleep verification was checking.
Restore strictness lost during the JMockit→Mockito migration: - PdmControllerTest: replace any(Class.class) with eq(J2735PdmRequest.class) for JsonUtils.fromJson stubs and drop @MockitoSettings(LENIENT) so default strict-mode enforces argument-class matching and unused-stubbing checks. - RsuHealthControllerTest: assert exactly one DefaultUdpTransportMapping, Snmp, and USM are constructed per heartBeat call, and verify that the constructed USM is registered via SecurityModels.addSecurityModel — the original JMockit Expectations enforced this implicitly but the migrated test only verified the final RsuSnmp.sendSnmpV3Request delegation.
- SnmpConfig: add missing Javadoc class comment - TimDeleteController: reformat to 2-space indent (project standard) - TimQueryController: reformat to 2-space indent (project standard)
- TimDeleteController: add Javadoc to constructor and deleteTim, break long log line, restore license-header trailing whitespace to match master (drops empty-line JavadocParagraph noise) - TimQueryController: add ending period to bulkQuery Javadoc summary
There was a problem hiding this comment.
Pull request overview
This PR removes the legacy JMockit-based testing approach (and its Java agent) and migrates affected tests to Mockito so that JaCoCo/Sonar coverage can include the previously excluded test suite. It also introduces a small SNMP construction seam (factory injection) to replace JMockit constructor-mocking needs.
Changes:
- Removed the JMockit Maven dependency and Surefire
-javaagentconfiguration. - Migrated a large set of tests from JMockit idioms to Mockito (including static and constructor mocking).
- Refactored SNMP session creation to be injectable via new factory interfaces / Spring configuration to support Mockito-based tests.
Reviewed changes
Copilot reviewed 49 out of 49 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pom.xml | Removes JMockit dependency and its Surefire agent configuration. |
| jpo-ode-svcs/src/test/java/us/dot/its/jpo/ode/traveler/TimQueryControllerTest.java | Migrates TIM query controller tests from JMockit to Mockito; introduces SnmpSessionFactory mocking. |
| jpo-ode-svcs/src/test/java/us/dot/its/jpo/ode/traveler/TimDepositControllerTest.java | Replaces JMockit static/constructor mocking with Mockito static mocking. |
| jpo-ode-svcs/src/test/java/us/dot/its/jpo/ode/traveler/TimDeleteControllerTest.java | Migrates TIM delete controller tests to Mockito and adapts to SnmpSessionFactory. |
| jpo-ode-svcs/src/test/java/us/dot/its/jpo/ode/stomp/WebSocketConfigTest.java | Converts JMockit verifications to Mockito verify(...). |
| jpo-ode-svcs/src/test/java/us/dot/its/jpo/ode/stomp/StompControllerTest.java | Migrates controller tests to Mockito + JUnit Jupiter. |
| jpo-ode-svcs/src/test/java/us/dot/its/jpo/ode/snmp/SnmpSessionTest.java | Updates SNMP session tests to Mockito; adds coverage for transport-construction failure via factory. |
| jpo-ode-svcs/src/test/java/us/dot/its/jpo/ode/rsuHealth/RsuSnmpTest.java | Migrates RSU SNMP tests to Mockito and adds interaction verification. |
| jpo-ode-svcs/src/test/java/us/dot/its/jpo/ode/rsuHealth/RsuHealthControllerTest.java | Replaces JMockit constructor/static mocking with Mockito mockConstruction/mockStatic. |
| jpo-ode-svcs/src/test/java/us/dot/its/jpo/ode/pdm/PdmUtilTest.java | Converts PDM util test to Mockito. |
| jpo-ode-svcs/src/test/java/us/dot/its/jpo/ode/pdm/PdmControllerTest.java | Migrates controller tests to Mockito static/constructor mocking for JsonUtils and SnmpSession. |
| jpo-ode-svcs/src/test/java/us/dot/its/jpo/ode/pdm/J2735PdmRequestTest.java | Removes JMockit usage; uses direct instantiation under JUnit Jupiter. |
| jpo-ode-svcs/src/test/java/us/dot/its/jpo/ode/SystemConfigTest.java | Updates assertions to JUnit Jupiter. |
| jpo-ode-svcs/src/test/java/us/dot/its/jpo/ode/OdePropertiesControllerTest.java | Migrates to Mockito + JUnit Jupiter and removes JMockit expectations. |
| jpo-ode-svcs/src/main/java/us/dot/its/jpo/ode/traveler/TimQueryController.java | Injects SnmpSessionFactory instead of direct new SnmpSession(...). |
| jpo-ode-svcs/src/main/java/us/dot/its/jpo/ode/traveler/TimDeleteController.java | Injects SnmpSessionFactory instead of direct new SnmpSession(...). |
| jpo-ode-svcs/src/main/java/us/dot/its/jpo/ode/snmp/TransportMappingFactory.java | New functional interface to abstract TransportMapping creation for testability. |
| jpo-ode-svcs/src/main/java/us/dot/its/jpo/ode/snmp/SnmpSessionFactory.java | New functional interface to abstract SnmpSession creation for controller testability. |
| jpo-ode-svcs/src/main/java/us/dot/its/jpo/ode/snmp/SnmpSession.java | Adds a transport-factory constructor to avoid constructor mocking. |
| jpo-ode-svcs/src/main/java/us/dot/its/jpo/ode/snmp/SnmpConfig.java | Registers a Spring bean providing the default SnmpSessionFactory. |
| jpo-ode-plugins/src/test/java/us/dot/its/jpo/ode/plugin/j2735/builders/WeatherProbeBuilderTest.java | Migrates static builder stubbing to Mockito static mocking. |
| jpo-ode-plugins/src/test/java/us/dot/its/jpo/ode/plugin/j2735/builders/VehicleSafetyExtensionsBuilderTest.java | Migrates builder static mocking to Mockito. |
| jpo-ode-plugins/src/test/java/us/dot/its/jpo/ode/plugin/j2735/builders/TravelerMessageFromHumanToAsnConverterTest.java | Removes JMockit logger mocking setup. |
| jpo-ode-plugins/src/test/java/us/dot/its/jpo/ode/plugin/j2735/builders/TimeStampConverterTest.java | Removes JMockit logger mocking setup; updates to JUnit Jupiter assertions. |
| jpo-ode-plugins/src/test/java/us/dot/its/jpo/ode/plugin/j2735/builders/SupplementalVehicleExtensionsBuilderTest.java | Migrates multiple static builder stubs to Mockito static mocking. |
| jpo-ode-plugins/src/test/java/us/dot/its/jpo/ode/plugin/j2735/builders/Position3DBuilderTest.java | Migrates static helper stubs to Mockito static mocking. |
| jpo-ode-plugins/src/test/java/us/dot/its/jpo/ode/plugin/j2735/builders/EventDescriptionBuilderTest.java | Migrates static helper stubs to Mockito static mocking. |
| jpo-ode-plugins/src/test/java/us/dot/its/jpo/ode/plugin/j2735/builders/BsmPart2ContentBuilderTest.java | Replaces JMockit capturing with managed Mockito static mocks and verification. |
| jpo-ode-plugins/src/test/java/us/dot/its/jpo/ode/plugin/j2735/builders/BsmCoreDataBuilderTest.java | Replaces JMockit capturing with managed Mockito static mocks. |
| jpo-ode-plugins/src/test/java/us/dot/its/jpo/ode/plugin/j2735/J2735VehicleStatusRequestTest.java | Removes JMockit; uses direct instantiation. |
| jpo-ode-plugins/src/test/java/us/dot/its/jpo/ode/plugin/j2735/J2735TransmissionAndSpeedTest.java | Removes JMockit; updates to JUnit Jupiter assertions. |
| jpo-ode-plugins/src/test/java/us/dot/its/jpo/ode/plugin/j2735/J2735TrailerUnitDescriptionTest.java | Removes JMockit; updates to JUnit Jupiter assertions. |
| jpo-ode-plugins/src/test/java/us/dot/its/jpo/ode/plugin/j2735/J2735TrailerDataTest.java | Removes JMockit; uses direct instantiation. |
| jpo-ode-plugins/src/test/java/us/dot/its/jpo/ode/plugin/j2735/J2735SupplementalVehicleExtensionsTest.java | Removes JMockit; uses direct instantiation. |
| jpo-ode-plugins/src/test/java/us/dot/its/jpo/ode/plugin/j2735/J2735SpeedProfileTest.java | Removes JMockit; uses direct instantiation. |
| jpo-ode-plugins/src/test/java/us/dot/its/jpo/ode/plugin/j2735/J2735SpecialVehicleExtensionTest.java | Removes JMockit; uses direct instantiation. |
| jpo-ode-plugins/src/test/java/us/dot/its/jpo/ode/plugin/j2735/J2735RTCMPackageTest.java | Removes JMockit; uses direct instantiation. |
| jpo-ode-plugins/src/test/java/us/dot/its/jpo/ode/plugin/j2735/J2735ProbeDataManagmentTest.java | Removes JMockit; uses direct instantiation. |
| jpo-ode-plugins/src/test/java/us/dot/its/jpo/ode/plugin/j2735/J2735PathHistoryPointTest.java | Removes JMockit; uses direct instantiation. |
| jpo-ode-plugins/src/test/java/us/dot/its/jpo/ode/plugin/j2735/J2735PathHistoryPointListTest.java | Removes JMockit; uses direct instantiation. |
| jpo-ode-plugins/src/test/java/us/dot/its/jpo/ode/plugin/j2735/J2735FullPositionVectorTest.java | Removes JMockit; uses direct instantiation. |
| jpo-ode-plugins/src/test/java/us/dot/its/jpo/ode/plugin/j2735/J2735EventDescriptionTest.java | Removes JMockit; uses direct instantiation. |
| jpo-ode-plugins/src/test/java/us/dot/its/jpo/ode/plugin/j2735/J2735EmergencyDetailsTest.java | Removes JMockit; uses direct instantiation. |
| jpo-ode-plugins/src/test/java/us/dot/its/jpo/ode/plugin/j2735/J2735ChoiceTest.java | Removes a JMockit-only exception test and updates to JUnit Jupiter assertions. |
| jpo-ode-plugins/src/test/java/us/dot/its/jpo/ode/plugin/j2735/J2735BsmTest.java | Removes JMockit; uses direct instantiation. |
| jpo-ode-plugins/src/test/java/us/dot/its/jpo/ode/plugin/j2735/DdsAdvisoryDetailsTest.java | Removes JMockit instantiation; uses direct instantiation (still has some legacy imports). |
| jpo-ode-plugins/src/test/java/us/dot/its/jpo/ode/plugin/generic/SituationDataWarehouseTest.java | Migrates to JUnit Jupiter assertions; removes JMockit. |
| jpo-ode-plugins/src/test/java/us/dot/its/jpo/ode/plugin/generic/RoadSideUnitTest.java | Migrates to JUnit Jupiter assertions; removes JMockit. |
| jpo-ode-core/src/test/java/us/dot/its/jpo/ode/snmp/SNMPTest.java | Migrates to JUnit Jupiter assertions; removes JMockit/JUnit4 patterns. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
73
to
75
| <version>3.0.8</version> | ||
| <type>pom</type> | ||
| </dependency> |
Comment on lines
+84
to
+88
| SnmpSession snmpSession = null; | ||
| try { | ||
| snmpSession = snmpSessionFactory.create(queryTarget); | ||
| snmpSession.startListen(); | ||
| } catch (IOException e) { |
Comment on lines
+123
to
+127
| ResponseEvent rsuResponse = null; | ||
| try { | ||
| rsuResponse = ss.set(pdu, ss.getSnmp(), ss.getTarget(), false); | ||
| } catch (IOException e) { | ||
| logger.error("Error sending TIM query PDU to RSU", e); |
Comment on lines
+76
to
80
| RsuSnmp.sendSnmpV3Request("127.0.0.1", "1.1", snmp, null); | ||
|
|
||
| RsuSnmp.sendSnmpV3Request("127.0.0.1", "1.1", mockSnmp, null); | ||
| // close() is unreachable when send() throws — guards against accidentally adding a finally block | ||
| verify(snmp, never()).close(); | ||
| } |
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
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.
PR Details
Description
This pull request removes JMockit from the project entirely and migrates every test that used it to Mockito, the mocking framework that ships with Spring Boot Test by default. The JMockit Maven dependency and its
-javaagentSurefire argument are deleted, and roughly forty test classes acrossjpo-ode-svcs,jpo-ode-core,jpo-ode-plugins, andjpo-ode-commonhave been rewritten to use Mockito idioms (@Mock,@InjectMocks,when(...).thenReturn(...),verify(...),mockStatic,mockConstruction).A small production change was needed to make one class testable without JMockit's constructor mocking:
SnmpSessionnow receives itsSnmpandTransportMappingcollaborators through newSnmpSessionFactoryandTransportMappingFactorySpring beans rather than constructing them directly. Two pre-existing tests that depended on JMockit's ability to mock unmockable third-party constructs (a logger field and anIOExceptionthrown from a final method) have been removed because they cannot be expressed in Mockito, and the scenarios they covered are not reachable through the public API.Related Issue
N/A.
Motivation and Context
JMockit has not had a meaningful release in years, and its bytecode-rewriting Java agent is incompatible with the JaCoCo agent that SonarCloud relies on for coverage. As long as JMockit was on the classpath, any test it touched was excluded from our SonarCloud coverage numbers, resulting in an artificially low and misleading coverage signal. Moving these tests to Mockito both removes a dead dependency and makes the affected code visible again in coverage analysis. Mockito is already a transitive dependency of
spring-boot-starter-test, so this change reduces our dependency footprint rather than expanding it.How Has This Been Tested?
The full Maven test suite (
mvn clean verify) was run locally and passed. CI runs the same suite on every push to this branch.Types of changes
Checklist:
ODE Contributing Guide