Skip to content

support parallel test execution #260

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Jannik2099
Copy link

This adds parallel test execution through the vscode TestController API. Sequential tests test(..., is_parallel = false) are respected.

Parallelism is set to the number of CPUs, if desired I could also add a config option.

I also fixed output formatting of test failures as vscode.TestRun.appendOutput always expects CLRF.

@Jannik2099
Copy link
Author

I couldn't help myself and just added test lookup functionality (right click on test -> Go to Test).

This leads to the source file that a test exe is built from. This only works for simple tests that are made up of one source file, but that should be the majority of tests.

@tristan957
Copy link
Member

@Jannik2099 do you think this PR is ready to go? Sorry it's taken me a long time to get to it

@Jannik2099 Jannik2099 force-pushed the parallel_tests branch 3 times, most recently from d70915c to ecdb6f2 Compare October 31, 2024 19:15
@Jannik2099
Copy link
Author

Sorry for the delay, I managed to break my finger which made interfacing with a keyboard... difficult.
Programming is a dangerous endeavour!

I addressed all your points.

@tristan957
Copy link
Member

I guess this slipped my mind when cutting the new release. Is there any way you could rebase on main, so I can get this for 1.28?

Signed-off-by: Jannik Glückert <[email protected]>
Signed-off-by: Jannik Glückert <[email protected]>
this is required when targets exist twice,
e.g. due to subprojects

Signed-off-by: Jannik Glückert <[email protected]>
@Jannik2099
Copy link
Author

No worries, I rebased & tested that things still work as expected.

@tristan957
Copy link
Member

Thanks for your work. Will review next week.

@Jannik2099
Copy link
Author

ping?

Copy link
Member

@tristan957 tristan957 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some initial comments

Comment on lines +161 to +166
"mesonbuild.testJobs": {
"type": "integer",
"default": -1,
"minimum": -1,
"description": "Specify the maximum number of tests executed in parallel. -1 for number of CPUs, 0 for unlimited."
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of magic integers can we use strings to indicate unlimited and ncpus?

Comment on lines +8 to +9
// this is far from complete, but should suffice for the
// "test is made of a single executable is made of a single source file" usecase.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you going to complete this TODO

Comment on lines +64 to +76
function testAdder(test: vscode.TestItem) {
const mesonTest = mesonTests.find((mesonTest) => {
return mesonTest.name == test.id;
})!;
if (mesonTest.is_parallel) {
parallelTests.push(test);
} else {
sequentialTests.push(test);
}
// this way the total number of runs shows up from the beginning,
// instead of incrementing as individual runs finish
run.enqueued(test);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make this an arrow function?

const xyz = ...

const buildDir = workspaceState.get<string>("mesonbuild.buildDir")!;
const mesonTests = await getMesonTests(buildDir);

function testAdder(test: vscode.TestItem) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment about what this function does?

Comment on lines +111 to +114
const running_tests: Promise<void>[] = [];
const max_running: number = (() => {
const jobs_config = extensionConfiguration("testJobs");
switch (jobs_config) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use camel case for naming

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants