You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Test Performance: Avoid repeatedly creating ExecutorService in ExpectTest (RCTP) #1624
I have analyzed the source code of core/src/test/java/com/google/common/truth/ExpectTest.java and identified a potential performance improvement regarding thread pool usage.
Currently, several test methods instantiate their own ExecutorService (via newFixedThreadPool or newSingleThreadExecutor) and shut it down at the end of the method.
Locations:
Line 187 (in bash method): ExecutorService executor = newFixedThreadPool(10);
Line 198 (in failWhenCallingThatAfterTest method): ExecutorService executor = newSingleThreadExecutor();
Line 211 (in failWhenCallingFailingAssertionMethodAfterTest method): ExecutorService executor = newSingleThreadExecutor();
Analysis (RCTP - Repeatedly Create Thread Pool) Initializing a new thread pool for each test execution incurs the overhead of thread creation, OS resource allocation, and subsequent teardown. While this provides test isolation, it contributes to slower test suite execution times, specifically due to the context switching and initialization costs associated with ThreadPoolExecutor.
If these tests are run frequently (e.g., in CI/CD pipelines or as part of a larger suite), this overhead accumulates.
Suggested Improvement Consider refactoring ExpectTest to use a shared ExecutorService instance. This could be achieved by:
Initializing a static ExecutorService in a @BeforeClass method.
This approach would reuse threads across the test methods (bash, failWhenCallingThatAfterTest, etc.), reducing the I/O and CPU overhead associated with repeated thread pool creation.
Code Example (Current vs Suggested):
Current: @test
public void bash() throws Exception {
ExecutorService executor = newFixedThreadPool(10); // Created every time
// ...
executor.shutdown();
}
Hi, I'd like to work on this issue. I'll refactor ExpectTest.java to use a shared ExecutorService initialized in @BeforeClass and torn down in @afterclass. I'll submit a PR shortly.
I reviewed the issue suggestion but identified that sharing a newFixedThreadPool(10) across single threaded tests would be wasteful since failWhenCallingThatAfterTest and failWhenCallingFailingAssertionMethodAfterTest only need a single thread. Instead I scoped the shared executor to only those two tests using newSingleThreadExecutor() leaving bash() unchanged as it is self contained. PR linked above.
I don't think we need to worry about this unless you can provide a benchmark showing that this is a significant bottleneck in the test suite. We don't optimize tests for performance at the cost of readability and test isolation.
Understood, thanks for the feedback! I’ll keep the performance vs readability tradeoff in mind for future contributions. This was my first open source contribution attempt ever so I appreciate the guidance looking forward to contributing more!
Many years ago, I helped another beginner with getting into open source, so you may find my thoughts from back then helpful.
In my experience, Guava is hard to contribute to because they hold such a high bar for code contributions. So, you're more likely to see success with other projects like Apache Commons or any of these other awesome-java projects. Alternatively, if you want to persevere with Guava, you may find success contributing to discussions here on the GitHub issue tracker.
I love software testing as I am a software tester right now but I also really like development so to be honest truth has more glamor I will try what you said thank you for the advice!
Activity
Aaron-PintoTamayo commentedon Mar 24, 2026
Hi, I'd like to work on this issue. I'll refactor ExpectTest.java to use a shared ExecutorService initialized in @BeforeClass and torn down in @afterclass. I'll submit a PR shortly.
Aaron-PintoTamayo commentedon Mar 24, 2026
I reviewed the issue suggestion but identified that sharing a newFixedThreadPool(10) across single threaded tests would be wasteful since failWhenCallingThatAfterTest and failWhenCallingFailingAssertionMethodAfterTest only need a single thread. Instead I scoped the shared executor to only those two tests using newSingleThreadExecutor() leaving bash() unchanged as it is self contained. PR linked above.
chaoren commentedon Mar 24, 2026
I don't think we need to worry about this unless you can provide a benchmark showing that this is a significant bottleneck in the test suite. We don't optimize tests for performance at the cost of readability and test isolation.
Aaron-PintoTamayo commentedon Mar 24, 2026
Understood, thanks for the feedback! I’ll keep the performance vs readability tradeoff in mind for future contributions. This was my first open source contribution attempt ever so I appreciate the guidance looking forward to contributing more!
jbduncan commentedon Mar 24, 2026
@Aaron-PintoTamayo Welcome!
Many years ago, I helped another beginner with getting into open source, so you may find my thoughts from back then helpful.
In my experience, Guava is hard to contribute to because they hold such a high bar for code contributions. So, you're more likely to see success with other projects like Apache Commons or any of these other
awesome-javaprojects. Alternatively, if you want to persevere with Guava, you may find success contributing to discussions here on the GitHub issue tracker.I hope this helps!
cpovirk commentedon Mar 25, 2026
(And Truth is like Guava but without the glamour if you get your submission merged... :))
(Come to think of it: It wouldn't be a bad idea to use
try-with-resource for the executors that we do create, if you want to try that.)Aaron-PintoTamayo commentedon Mar 25, 2026
I love software testing as I am a software tester right now but I also really like development so to be honest truth has more glamor I will try what you said thank you for the advice!
Aaron-PintoTamayo commentedon Mar 26, 2026
Submitted PR #1640 with the try-with-resources refactor as suggested. Thanks again for the tip @cpovirk!