#18200 closed defect (fixed)
[PATCH RFC] Mocking of private methods prohibited in JMockit 1.47
| Reported by: | floscher | Owned by: | ris |
|---|---|---|---|
| Priority: | normal | Milestone: | 20.11 |
| Component: | Unit tests | Version: | |
| Keywords: | jmockit | Cc: |
Description
Starting with JMockit 1.47 the mocking of private methods is no longer allowed.
The methods in WindowlessMapViewStateMocker have to be modified when you want to update JMockit to a newer version:
Attachments (2)
Change History (25)
comment:1 by , 5 years ago
| Owner: | changed from to |
|---|
comment:2 by , 5 years ago
| Component: | Core → Unit tests |
|---|---|
| Keywords: | jmockit added |
comment:3 by , 5 years ago
comment:4 by , 5 years ago
Well https://github.com/jmockit/jmockit1/commit/b864642695bc48da5c210984aa143cb0f80eb4e0 is the commit in question and if we're desperate for a new version before this is resolved we can always maintain a very minor fork with that patch dropped. Unfortunately the reason for the removal appears to be a dogmatic belief about testing methodologies, no real technical reason.
comment:5 by , 5 years ago
There was no good reasons neither to delete our comments when we reported him bug reports... We're not in a hurry, we can take the necessary time to avoid having to maintain a fork. Probably making methods protected would be enough?
comment:6 by , 5 years ago
Haha I see the guy is still a moron with everyone disagreeing with him: https://github.com/jmockit/jmockit1/issues/629 I guess you won't have much success with the issue you just opened :D
comment:7 by , 5 years ago
Someone created a very nice ticket explaining why this is needed in real life industrial projects: https://github.com/jmockit/jmockit1/issues/605
by , 4 years ago
| Attachment: | 18200.patch added |
|---|
Modify methods to be package instead of private, indicate via javadoc that they are visible for JMockit, update JMockit to 1.49
comment:8 by , 4 years ago
| Summary: | Mocking of private methods prohibited in JMockit 1.47 → [PATCH RFC] Mocking of private methods prohibited in JMockit 1.47 |
|---|
The patch I just attached just modifies methods to be usable for JMockit 1.47+. This is for JUnit5 -- JMockit 1.46 and older don't properly clean up their mocks on JUnit >= 5.5 (JMockit 1.46 works on JUnit 5.4).
comment:10 by , 4 years ago
| Milestone: | → 20.10 |
|---|
comment:11 by , 4 years ago
| Resolution: | fixed |
|---|---|
| Status: | closed → reopened |
This broke 8 unit tests: https://josm.openstreetmap.de/jenkins/job/JOSM/lastCompletedBuild/jdk=JDK8/testReport/
java.lang.IllegalArgumentException: Invalid Class argument for partial mocking (use a MockUp instead): class org.openstreetmap.josm.tools.PlatformManager
follow-up: 14 comment:12 by , 4 years ago
Only breaking 8 tests with this update is great news IMHO.
follow-up: 21 comment:14 by , 4 years ago
Replying to ris:
Only breaking 8 tests with this update is great news IMHO.
In other news, I broke tests again. :(
On a more serious note, it looks very simple to fix. I attached the current proof-of-concept fix (4/8 failures) so that other people can test and make certain it works for them.
Other things to note: I switched HelpBrowserTest to only use JUnit5 methods. I'll be doing the same for the other tests I modify.
To test the attached patch (18200.HelpBrowserTest.patch), apply the patch and then run the following:
ant test-clean test '-Ddefault-junit-includes=**/HelpBrowserTest.class'
comment:19 by , 4 years ago
comment:21 by , 4 years ago
Replying to taylor.smock:
Replying to ris:
Only breaking 8 tests with this update is great news IMHO.
In other news, I broke tests again. :(
No worries. Here no user facing functionality was broken. And sometimes breaking tests give a sense of safety (proving that they really find bugs/regressions)… :-)
comment:22 by , 4 years ago
| Resolution: | → fixed |
|---|---|
| Status: | reopened → closed |
Christ.