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

Implement concurrent limits based on item marks #47

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

arg0d
Copy link

@arg0d arg0d commented Sep 10, 2023

This change enables users to limit individual test concurrency based on mark configuration. For example, lets say that there are 10 tests, and 8 of them can be executed concurrently, but 2 of them use some shared resource and can't be executed concurrenctly. By using the new option --max-asyncio-tasks-by-mark, its now possible to configure the task scheduler to execute the 2 tasks in series, but the remaining tasks 8 tasks would still be scheduled concurrently.

PS
Master branch code was not formatted, look at specific commit diff to filter out code formatting changes.

@arg0d arg0d force-pushed the concurrent-mark-limits branch 4 times, most recently from 815e7bc to 00aab47 Compare September 10, 2023 16:07
@arg0d arg0d marked this pull request as draft September 11, 2023 06:07
@arg0d
Copy link
Author

arg0d commented Sep 11, 2023

This morning I realized there are still a couple of things that need to be implemented.

  • parsing spaces in key=value pairs
  • limit concurrency for remaining tasks that haven't matched any mark limit. This is different from --max-asyncio-tasks, because its desirable to configure how many remaining unmatched tasks can run concurrently (e.g. 1), while the total number of tasks might be greater (e.g. foo=1, bar=1, remainder=1, so --max-asyncio-tasks should be greater than 3 to run all 3 groups concurrently).

@arg0d arg0d force-pushed the concurrent-mark-limits branch 7 times, most recently from 5f9ddb5 to df68d87 Compare September 13, 2023 16:01
@arg0d arg0d marked this pull request as ready for review September 13, 2023 16:01
@arg0d
Copy link
Author

arg0d commented Sep 13, 2023

@willemt could you take a look at this? I think I'm pretty happy with the changes at this point. These are the new concurrency control cases:

  • control individual marks. 1 concurrent test marked with foo will run, and 2 concurrent tests marked with bar will run. Since the total limit is 10, there will be 6 additional tests with other marks running concurrently. At most 10 tests can run.

    --max-asyncio-tasks=10
    --max-asyncio-tasks-by-mark "foo=1 bar=2"
    
  • control groups of marks. Only 1 test marked with either foo or bar will run concurrently, 9 other tests will run concurrently.

    --max-asyncio-tasks=10
    --max-asyncio-tasks-by-mark "foo,bar=1"
    
  • control the remainder of unmatched marks, 2 tests marked with foo will run concurrently, 2 tests marked with bar will run concurrently, and the remaining tests will run in sequence.

    --max-asyncio-tasks=10
    --max-asyncio-tasks-by-mark "foo=2 bar=2"
    --max-asyncio-tasks-by-mark-remainder=1
    
  • control total concurrent tests. At most 2 tests marked with foo will run concurrently, and at most 2 tests marked with bar will run concurrently. Both foo and bar could run concurrently at some point, but the total limit of concurrent tasks is 2 across both foo and bar.

    --max-asyncio-tasks=2
    --max-asyncio-tasks-by-mark "foo=2 bar=2"
    

PS
Thanks for starting this project, I'm surprised its not more well known.

This change enables users to limit individual test concurrency based
on mark configuration. For example, lets say that there are 10 tests,
and 8 of them can be executed concurrently, but 2 of them use some
shared resource and can't be executed concurrenctly. By using the new
option `--max-asyncio-tasks-by-mark`, its now possible to configure
the task scheduler to execute the 2 tasks in series, but the remaining
tasks 8 tasks would still be scheduled concurrently.
@willemt
Copy link
Owner

willemt commented Nov 30, 2023

this looks great thanks!

because it makes changes to the core bits I want to first see if it's possible if I can refactor this PR into a plugin via apluggy (#52). I'm sensing I might need to expose some hooks

@arg0d
Copy link
Author

arg0d commented Nov 30, 2023

Its been a while, but if I remember correctly, I haven't used any new APIs that weren't used before. I added some new configuration values, and updated how tasks are scheduled.

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