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

Fix parallel invocation and also a couple typos per the spellchecker. #20

Merged
merged 1 commit into from
Oct 22, 2022

Conversation

binaryphile
Copy link

@binaryphile binaryphile commented Oct 20, 2022

I noticed that the tests want to be run in parallel since the test function calls #Parallel. However, this only allows the top-level function to run concurrently. Since there are no other top-level functions in the test, there's no effect and the subtests run serially.

While you still want to call #Parallel at the beginning of the function (so you can write other test functions and have them run parallel too), you need to call it in the #Run closure as well. Each subtest then runs in parallel. As it turns out, the tests are so fast that it makes no difference anyway, but I thought you should see the way to get the subtests running as well. Honestly, dropping concurrency is probably simpler, so feel free to do whatever with this PR.

Some refs:

https://gist.github.com/posener/92a55c4cd441fc5e5e85f27bca008721
https://github.com/moricho/tparallel

(Forgive the spelling corrections, my IDE makes the suggestions and I just accepted them.)

@bombsimon
Copy link
Owner

(Forgive the spelling corrections, my IDE makes the suggestions and I just accepted them.)

Nothing to be sorry for, it's appreciated! 😄

@bombsimon bombsimon merged commit c159d8b into bombsimon:main Oct 22, 2022
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