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

Use release for compliance tests #892

Closed
wants to merge 1 commit into from
Closed

Conversation

squell
Copy link
Member

@squell squell commented Oct 22, 2024

I found this branch by @pvdrz lying around; this seems like it can be merged?

@squell squell added minor minor issue, PR without an issue test-framework labels Oct 22, 2024
@squell squell requested a review from japaric October 22, 2024 15:07
@squell squell enabled auto-merge (rebase) October 22, 2024 15:07
@bjorn3
Copy link
Collaborator

bjorn3 commented Oct 23, 2024

If this is for performance reasons, I don't think this will help a whole lot. I suspect that most of the time is spent at overhead of docker itself and also some time in all the sleep calls that exist all around the compliance tests for synchronization purposes. While a test is sleeping, no other test is able to make use of the unused cpu core.

@squell
Copy link
Member Author

squell commented Oct 23, 2024

If this is for performance reasons, I don't think this will help a whole lot. I suspect that most of the time is spent at overhead of docker itself and also some time in all the sleep calls that exist all around the compliance tests for synchronization purposes. While a test is sleeping, no other test is able to make use of the unused cpu core.

I want sudo-rs to be blazingly fast! ⚡ 😉

No, it is for "good practice" reasons (this is less relevant for safe Rust perhaps, but in many languages you are eventually going to have a bad day if you only ever test in debug mode--I can share anecdotes). Of course to really stick to this idea we should then also consider compiling the actual release with overflow checks and debug assertions on as well. I wouldn't mind that actually.

I remember we discussed this last year but don't recall the outcome.

@squell
Copy link
Member Author

squell commented Oct 23, 2024

Come to think of it, if we really want to get the full benefit we should put the exact release build through its paces in the compliance test, of course. I.e. build it using the util/build-release.sh script and then move the binaries into the test harness. Probably that's why we didn't merge this a year ago :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor minor issue, PR without an issue test-framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants