-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Implement Parallel Method Execution in JUnit-Vintage engine #4242
base: main
Are you sure you want to change the base?
Changes from 3 commits
0cd9f4d
be5234f
0b70615
6e679dc
e92e834
96951ce
78c5c80
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
/* | ||
* Copyright 2015-2025 the original author or authors. | ||
* | ||
* All rights reserved. This program and the accompanying materials are | ||
* made available under the terms of the Eclipse Public License v2.0 which | ||
* accompanies this distribution and is available at | ||
* | ||
* https://www.eclipse.org/legal/epl-v20.html | ||
*/ | ||
|
||
package org.junit.vintage.engine.descriptor; | ||
|
||
import static org.apiguardian.api.API.Status.INTERNAL; | ||
|
||
import org.apiguardian.api.API; | ||
|
||
/** | ||
* Represents a strategy for scheduling when individual test methods | ||
* should be run (in serial or parallel) | ||
* | ||
* @since 5.13 | ||
*/ | ||
@API(status = INTERNAL, since = "5.13") | ||
public interface RunnerScheduler { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can't we use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Directly using However, when running the
So, I’m considering restoring it again. What are your thoughts on that? 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I pushed a commit that fixes that. Instead of referencing |
||
/** | ||
* Schedule a child statement to run | ||
*/ | ||
void schedule(Runnable childStatement); | ||
|
||
/** | ||
* Override to implement any behavior that must occur | ||
* after all children have been scheduled (for example, | ||
* waiting for them all to finish) | ||
*/ | ||
void finished(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think shutting down the thread pool here is not correct because this will be called for each runner plus we're also using it if
classes
istrue
as well.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with the comment you provided.
Based on that, I removed the
shutdown
and only used theawaitTermination
method.Then, I added a conditional statement to check if it completes within the given time. e92e834
Upon testing, I found that not all tests finished within the specified time.
As a result, I implemented the
finished
method to do nothing, but this caused the assertion comparing the start and end times to fail.Could you provide a hint about what kind of work might need to be done inside the
finished
method?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to wait for all futures scheduled for a particular runner. I have addressed that in the commit I've pushed to your branch just now.