Skip to content
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

Reduce build duration by lowering Mockito usage #2707

Open
jkronegg opened this issue Mar 16, 2023 · 8 comments · May be fixed by #2767
Open

Reduce build duration by lowering Mockito usage #2707

jkronegg opened this issue Mar 16, 2023 · 8 comments · May be fixed by #2767
Assignees
Labels
🏦 debt Tech debt 🙏 help wanted Help wanted - not prioritized by core team

Comments

@jkronegg
Copy link
Contributor

jkronegg commented Mar 16, 2023

🤔 What's the problem you've observed?

Mockito has a impact on the unit test execution time (seen using IntelliJ Profiler flame graph):

project test duration [s] mockito contribution
cucumber-junit 4.4 31% = 1.3 s
cucumber-core 7.5 10% = 0.7 s
cucumber-java 2.8 24% = 0.6 s
cucumber-java8 1.8 27% = 0.5 s
cucumber-spring 4.5 13% = 0.5 s
cucumber-testng 1.5 20% = 0.3 s
cucumber-guice 1.8 15% = 0.3 s
datatable 0.5 (unused dependency) 0% = 0 s

Total build duration: 3min17 = 197 seconds
Total test duration : 23.5 seconds
Total Mockito contribution : 4.2 seconds (18% of tests, 2% of build duration)

By reducing the usage of Mockito, the test duration can be lowered with two advantages:

  • this gives the developer a shorter feedback loop.
  • the energy consumption is lower (green IT), which is important because the cucumber-jvm project is built often (due to PR and renovate bot). 2% is not a lot, but on the long term, it makes a difference.

✨ Do you have a proposal for making it better?

I didn't check in details, but for example in JavaBackendTest, the @Mock ObjectFactory and Glue could be replaced by fake implementations, which could reduce the class test duration by about 80%.

@jkronegg jkronegg added the 🏦 debt Tech debt label Mar 16, 2023
@mpkorstanje
Copy link
Contributor

mpkorstanje commented Mar 19, 2023

Yes. This makes sense. I would agree to Mockito replacements if:

  1. Stubs implementations are declared in the test class.
  2. Stub implementations are simple.

This will likely result in some duplication but that is an acceptable tradeoff.

@jkronegg jkronegg added good first issue Good for newcomers 🙏 help wanted Help wanted - not prioritized by core team labels Mar 22, 2023
@jkronegg jkronegg self-assigned this Apr 21, 2023
@jkronegg jkronegg linked a pull request Jun 6, 2023 that will close this issue
7 tasks
@jkronegg jkronegg removed the good first issue Good for newcomers label Jun 6, 2023
jkronegg pushed a commit that referenced this issue Aug 4, 2023
@CharlesLgn
Copy link

Does someone made a study to use another mock library to see if there will be better performance ? (EasyMock/JMockit/...)

@jkronegg
Copy link
Contributor Author

I didn't tried with other mock library. The proposed solution #2767 replaces mockito by stub and fake implementations, which leads to a 7% improvement of the build time (that is, much more than the 2% expected).

@CharlesLgn
Copy link

I didn't tried with other mock library. The proposed solution #2767 replaces mockito by stub and fake implementations, which leads to a 7% improvement of the build time (that is, much more than the 2% expected).

I understand the principle of using fake implementation. However, I think that if you have an interface with 25 methods, the test class will be poluate with a lot of fake implementation, with also a lot unuse code.

That is why I questioned the use of other library

@jkronegg
Copy link
Contributor Author

I agree that readability is generally lower when using stub classes with a lot of methods... This may be improved by having specific stub classes that are shared between test cases, but @mpkorstanje wanted stubs implementations to be declared in the test class.

I didn't found a lot of mocking libraries performance comparisons, but according to mockito/mockito#3288, mockito and Easymock have more or less the same performance.

@CharlesLgn
Copy link

CharlesLgn commented Jan 15, 2025

I didn't found a lot of mocking libraries performance comparisons, but according to mockito/mockito#3288, mockito and Easymock have more or less the same performance.

As I understand, this is not what they said.

mockito/mockito#3288 (comment)

So you see that EasyMock shows 100x less average execution time. And that's very strange. Can you please check my benchmarks? Have I done something wrong? Or how can you explain that?

the person opening the ticket says that EasyMock shows 100x less average execution time.

EasyMock

Callable mock;

@Setup
public void setup() throws Exception {
        mock = EasyMock.mock(Callable.class);

	EasyMock.expect(mock.call()).andReturn("1").times(1, Integer.MAX_VALUE);
	EasyMock.replay(mock);
}

@Benchmark
public Object testEastMock() throws Exception {
	return mock.call();
}

Mockito

Callable mock;

@Setup
public void setup() throws Exception {
	mock = mock(Callable.class, withSettings().mockMaker(MockMakers.INLINE));
	when(mock.call()).thenReturn("1");
}

@Benchmark
public Object testMockitoInline() throws Exception {
	return mock.call();
}
Benchmark                              Mode  Cnt   Score     Error    Units
MockitoBenchmarking.testEastMock       avgt    5    46.058 ±   0.443  ns/op
MockitoBenchmarking.testMockitoInline  avgt    5  4428.439 ± 356.722  ns/op

but maybe I did not understand what @sergey-morenets is saying 😕

@mpkorstanje
Copy link
Contributor

mpkorstanje commented Jan 15, 2025

Alright folks take it easy on the bike-shedding. 😉

As is, I've picked up a few parts of this PR. But overall it is quite boring to review and quite important to get right. So it does not seem to end on top of my priority queue. Switching mocking frameworks would be equally tedious to review so I'd like to avoid that too.

@jkronegg my appologies for that.

@jkronegg
Copy link
Contributor Author

As time passes, I'm less convinced that gaining 2-7% on build time is really important. It's a lot of development and review effort (either with another mocking framework or without mocking framework).

@CharlesLgn yes, there seems to a 100x difference when calling Easymock vs Mockito methods (I don't remember why I wrote that the performance is the same; maybe because the methods are in practice called only a few times?🤔).

@mpkorstanje I agree that the development effort would be better invested on #2902 (which makes run Cucumber 2x faster) 😉. I can work on it if you want (I know you would prefer to manage the issue with a full rewrite of cucumber-java8 but 2x faster with just some caching looks good to me).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏦 debt Tech debt 🙏 help wanted Help wanted - not prioritized by core team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants