Modify

Opened 10 months ago

Last modified 10 months ago

#23729 new task

Investigate moving from jmockit to something actively maintained

Reported by: taylor.smock Owned by: team
Priority: normal Milestone: Longterm
Component: Core Version:
Keywords: Cc:

Description (last modified by taylor.smock)

jmockit is no longer maintained. While I don't think it is going to break anytime soon, I do think it is highly likely to break in the next few years. Specifically, it (apparently) uses a customized fork of ASM, and that is highly likely to break at some point.

I'd rather start working to replace it now (when it isn't broken) than have to replace it suddenly when something breaks it.

There was some discussion in #16010 about whether or not we should try using mockito instead of jmockit. Of note:

  • jmockit only had one developer who appears to have abandoned the project
  • jmockit supports "global" mocks
    • We'd apparently have to do a lot of work to get mockito working properly (specifically, we'd have to fiddle with dependency injection)
    • mockito does support thread-local mocks now (apparently), so this might be "good enough" since we really only have three threads we care about most of the time (MainApplication.worker, EDT, and the JUnit thread). See MockedStatic.
  • jmockit supports "private" method mocks (but so does mockito + powermock; powermock isn't supported anymore though)
    • We mostly need private method mocking due to some of our tests hitting UI codepaths. Maybe we should fix those? Maybe with something like BooleanSupplier/IntSupplier for answers to UI questions?

Potential plans of action:

  • Add dependency on mockito and start moving tests to it
  • Switch jmockit from original author to hazendaz
  • Remove need for (most) mocks by adding parameters that will call the UI codepaths in the UI, but otherwise return a specified value

Example of the last point:

public static void combineWays(Collection<Way> ways) {
    // Do stuff
    // Then ask question
    if (ConditionalOptionPaneUtil.showConfirmationDialog(...)) {
        // Finish doing stuff
    }
}
// Versus
public static void combineWays(Collection<Way> ways) {
    combineWays(ways, () -> ConditionalOptionPaneUtil.showConfirmationDialog(...));
}
public static void combineWays(Collection<Way> ways, BooleanSupplier checkConfirmation) {
    // Do stuff
    // Then ask question
    if (checkConfirmation.getAsBoolean()) {
        // Finish doing stuff
    }
}

An important note is that none of the potential plans of action are exclusionary.

Attachments (0)

Change History (5)

comment:1 by taylor.smock, 10 months ago

Description: modified (diff)

comment:2 by stoecker, 10 months ago

There is a fork at ​https://github.com/hazendaz/jmockit1 (do we want to use that instead? they've integrated some of our PRs against jmockit and seem to be active)

Seems to me the most straight forward and easiest way.

Remove need for (most) mocks by adding parameters that will call the UI codepaths in the UI, but otherwise return a specified value

I don't like that much. It clutters the API for the sole purpose of tests.

in reply to:  2 comment:3 by taylor.smock, 10 months ago

Replying to stoecker:

Seems to me the most straight forward and easiest way.

It definitely is; I'm just worried about long-term problems. There is (still) only one "major" contributor to the jmockit fork, and he may lose interest in it as well.

What I'm tempted to do is add mockito as an additional test dependency and try moving as much stuff as possible to it. Yes, that would add a dependency to our test environment, but it wouldn't affect JOSM runtime size.

I don't like that much. It clutters the API for the sole purpose of tests.

Fair enough, with the caveat that it also makes it easier for plugins to use some functionality in ways that we do not anticipate. So it wouldn't be only for tests, but tests would be the primary beneficiary/driving factor.

comment:4 by stoecker, 10 months ago

Well, predicting the future wont work and doing work based on such a prediction is wasted effort. I've seen active projects die and slow-going projects surviving as well as all other forms. Active maintaining is only one of the factors for project survival.

If there is an easy replacement looking good at the moment use it and only go the harder way in replacing stuff when it becomes necessary.

comment:5 by taylor.smock, 10 months ago

Fair enough. With that said, mockito is very widely used, so if it does "go dark", I strongly suspect that a continuation will be done fairly quickly.

My problem is that we are currently using a "soft" fork of JMockit (see https://github.com/JOSM/jmockit1 ) since the jmockit author decided to remove private method mocking, which we needed at the time (and maybe still need?).

The problem with private method mocking is that it I believe it involves some unsupported trickery in the JVM (at least post JPMS). AFAICT, almost all mocking projects that are still supported don't support private method mocking. Since it isn't something that is widely used, I strongly suspect that at some point in time, it will become incredibly difficult to do.

Really, what I should do, is write a POC change for mockito and see if it will work.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The owner will remain team.
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from team to the specified user.
Next status will be 'needinfo'. The owner will be changed from team to taylor.smock.
as duplicate The resolution will be set to duplicate. Next status will be 'closed'. The specified ticket will be cross-referenced with this ticket.
The owner will be changed from team to anonymous. Next status will be 'assigned'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.