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

core: remove the shared state from the tests #34

Merged
merged 1 commit into from
Jan 25, 2023
Merged

Conversation

cvengler
Copy link
Member

This commit removes the shared state from the cgroup tests, by generating a unique random name for each and every cgroup.

Fixes #33

@cvengler cvengler requested a review from dadada January 17, 2023 17:12

/// Generates a name for the current cgroup
fn gen_name() -> String {
let val: u64 = rand::random();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use something that is reliably unique instead (e.g., a UUID). Or just the thread ID (gettid) of the test.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 for uuid. https://docs.rs/uuid/latest/uuid/ looks good at a first glance

Copy link
Member Author

Choose a reason for hiding this comment

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

I personally think that the UUID crate is overkill for a test dependency. 64-bit integers are totally sufficient in that case. I've added a loop on top of it, in order to avoid collisions, although the probability is with $2^{-64}$ very very low.

Copy link
Member Author

Choose a reason for hiding this comment

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

Beside this, the UUID crate seems to require an instance before you can generate UUIDs. This would already make the code less obvious in my opinion, compared to the anonymous integer generator.

dadada

This comment was marked as duplicate.

This commit removes the shared state from the cgroup tests, by generating a
unique random name for each and every cgroup.

Fixes #33
@sevenautumns sevenautumns merged commit 49f71f4 into main Jan 25, 2023
@cvengler cvengler deleted the cgroup-test-no-ss branch January 26, 2023 14:06
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.

Better test isolation
4 participants