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

tests: allow user to select tests via command line args #1211

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jonasnick
Copy link
Contributor

This PR introduces the ability for users to select specific tests to run via command line arguments. The aim is to eliminate the need to comment out parts of the tests.c file in order to speed up testing. The implementation uses similar command line flags as the benchmarks, for example:

./tests ecmult schnorrsig

The approach taken in this PR may not be the most straightforward, but it ensures backwards compatibility. The shell script located at https://gist.github.com/jonasnick/5e37248e3fa5911cd41e1da2f5f4e395 can be used to test the changes introduced in this PR. I'd be happy to add more flags if needed.

@real-or-random
Copy link
Contributor

Concept ACK

I still think it's an interesting idea for the future to look into existing test frameworks, which typically support running a subset of tests. (But that's no reason not to merge this, of course.)

@@ -28,7 +29,8 @@

#define CONDITIONAL_TEST(cnt, nam) if (COUNT < (cnt)) { printf("Skipping %s (iteration count too low)\n", nam); } else

static int COUNT = 64;
static const int DEFAULT_COUNT = 64;
static int COUNT = DEFAULT_COUNT;
Copy link
Contributor

Choose a reason for hiding this comment

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

MSVC doesn't like that line apparently because DEFAULT_COUNT is not a constant, which may be true by a strict reading of the standard...

Maybe try #defining DEFAULT_COUNT instead, or set COUNT = DEFAULT_COUNT; only at the beginning of main().

(By the way, this shows that CI on MSVC will timeout after 60 min in case of failure instead of just aborting. This will be fixed by e433034)

@sipa
Copy link
Contributor

sipa commented Feb 10, 2023

Concept ACK

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

In the commit 81f791e "tests: add "help" command line flag and output", shouldn't #include "cli_util.h" be added to all src/modules/*/bench_impl.h where the have_flag function is actually used?

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.

None yet

4 participants