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

Test that New(n) with n ≤ 0 will panic #10

Merged
merged 2 commits into from
Feb 8, 2021

Conversation

tklauser
Copy link
Member

@tklauser tklauser commented Feb 5, 2021

Make sure that constructing a worker pool with zero or negative n will
panic.

Also:

PASS
coverage: 100.0% of statements
ok  	github.com/cilium/workerpool	0.614s	coverage: 100.0% of statements

😉

Note: branch based on pr/kaworu/test-cleanup (#9) to avoid merge conflicts. GitHub should automatically rebase this one once #9 is merged.

@tklauser tklauser requested review from kaworu and rolinh February 5, 2021 15:09
@tklauser tklauser mentioned this pull request Feb 5, 2021
rolinh
rolinh previously approved these changes Feb 5, 2021
@rolinh rolinh added the dont-merge/blocked Another PR must be merged before this one. label Feb 5, 2021
kaworu
kaworu previously approved these changes Feb 8, 2021
Copy link
Member

@kaworu kaworu left a comment

Choose a reason for hiding this comment

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

LGTM!

@kaworu kaworu force-pushed the pr/kaworu/test-cleanup branch from b7a377f to 04b98b6 Compare February 8, 2021 09:37
Base automatically changed from pr/kaworu/test-cleanup to master February 8, 2021 09:42
@kaworu kaworu dismissed stale reviews from rolinh and themself February 8, 2021 09:42

The base branch was changed.

@kaworu kaworu added needs-rebase This PR needs to be rebased because it has merge conflicts. and removed dont-merge/blocked Another PR must be merged before this one. labels Feb 8, 2021
Make sure that constructing a worker pool with zero or negative n will
panic.

Signed-off-by: Tobias Klauser <tobias@isovalent.com>
Signed-off-by: Tobias Klauser <tobias@isovalent.com>
@rolinh rolinh force-pushed the pr/tklauser/test-new-panic branch from 41e22ae to f02e9f5 Compare February 8, 2021 10:59
@rolinh rolinh removed the needs-rebase This PR needs to be rebased because it has merge conflicts. label Feb 8, 2021
@rolinh rolinh merged commit 1114317 into master Feb 8, 2021
@rolinh rolinh deleted the pr/tklauser/test-new-panic branch February 8, 2021 11:01
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.

3 participants