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(go_app): fix CPU usage by test app #2809

Merged
merged 4 commits into from
Apr 2, 2024
Merged

Conversation

silvestre
Copy link
Member

@silvestre silvestre commented Mar 28, 2024

Issue

The test app was not using the requested amount of CPU in all cases.

Cause

The main issue was that the main "busy loop", tasked with generating the synthetic CPU load, was interrupted too frequently to check if the overall test was done. This made it inaccurate.

Fix

The busy loop now runs uninterrupted for at least a second, which makes the check infrequent enough, so that it doesn't impact the CPU load.

Additionally, the unit tests were fixed, so that they actually test that the CPU is used as specified.

@silvestre silvestre added the allow-acceptance-tests This label needs to be added to enable the acceptance tests to run. label Mar 28, 2024
@silvestre silvestre force-pushed the fix-cpu-usage-test-app branch from 334f2cc to 34f9d57 Compare March 28, 2024 16:21
@silvestre silvestre force-pushed the fix-cpu-usage-test-app branch from 34f9d57 to 1400008 Compare March 28, 2024 16:36
@silvestre silvestre marked this pull request as ready for review March 28, 2024 18:06
@silvestre silvestre requested a review from geigerj0 March 28, 2024 18:06
@silvestre silvestre added the bug label Apr 2, 2024
Copy link
Contributor

@geigerj0 geigerj0 left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this so fast 🚀

- `min` is now builtin
- `RWMutex` captures intent better
Copy link

sonarqubecloud bot commented Apr 2, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@silvestre silvestre merged commit b37669f into main Apr 2, 2024
38 checks passed
@silvestre silvestre deleted the fix-cpu-usage-test-app branch April 2, 2024 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
allow-acceptance-tests This label needs to be added to enable the acceptance tests to run. bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants