Skip to content

util-cpu: add a mockup value for unittests v1 #13645

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.

Sign up for GitHub

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

Closed
lukashino wants to merge 1 commit into from
Closed

util-cpu: add a mockup value for unittests v1 #13645

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/util-cpu.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,10 @@ uint16_t UtilCpuGetNumProcessorsConfigured(void)
*/
uint16_t UtilCpuGetNumProcessorsOnline(void)
{
#ifdef UNITTESTS
return 40; // A mockup value for unittests
#endif
Comment on lines +110 to +112

@jasonish jasonish Jul 24, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How does this affect thing if I build with unittests, but then run live, on a box with 2 cores?

@lukashino lukashino Jul 24, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was afraid this case should be supported as well 😅 alternatively the tests can check real CPU core presence and skip tests based on the environment and requirements. This is implemented in the other PR#13644

@lukashino lukashino Jul 25, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If it is a usual and still working case, then surely this would not be acceptable, and I would only propose skipping the tests on insufficient machine capabilities.

@catenacyber catenacyber Jul 28, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wonder if rather ThreadingAffinityTest03 and such should be skipped if the requirements are not met

@lukashino lukashino Jul 29, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, I would be in favor of that approach as is implemented in #13644 in https://github.com/OISF/suricata/pull/13644/files#diff-1700c8224734e17b7f5fc2cb71b7c53c110438d178cf44ae0d35312c32b3e229R2060

Alternatively, we can add another switch to e.g. configure script that would enable unittest mockups and HW dependent tests would be otherwise skipped? configure --enable-unittests --enable-unittest-mockups

But I still prefer the real HW test as we can generally limit the tests to, e.g., 4 cores (or other limited HW resources).

@catenacyber catenacyber Jul 30, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So, should you close this PR in favor of #13644 and such ? (if you prefer the other PR as I do)

@lukashino lukashino Aug 6, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok, sounds good.

@satta satta Sep 25, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note that this actually breaks Debian package tests which run the unit tests on i386 platforms where the test machines have less than 4 CPUs: https://ci.debian.net/packages/s/suricata/testing/i386/64736439/

I also have a patch which determines at runtime whether UtilCpuGetNumProcessorsOnline() is called from a unit test or not, and which returns a hardcoded mock value in that case (See 994d01c) -- but would also be fine with skipping the tests.

For Debian I would just disable these tests so they don't hold up testing transitions.


#ifdef SYSCONF_NPROCESSORS_ONLN_COMPAT
long nprocs = -1;
nprocs = sysconf(_SC_NPROCESSORS_ONLN);
Expand Down
Loading