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.

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

ztest: add a ztest shell command #65437

Closed
wants to merge 2 commits into from
Closed

ztest: add a ztest shell command #65437

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
22669fd
ztest: add a ztest shell command
cfriedt Nov 18, 2023
4d3b79b
tests: ztest: add testsuite for ztest shell
cfriedt Nov 22, 2023
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
1 change: 1 addition & 0 deletions subsys/testsuite/ztest/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ zephyr_library_sources(
src/ztest_rules.c
)
zephyr_library_sources_ifdef(CONFIG_ZTEST_MOCKING src/ztest_mock.c)
zephyr_library_sources_ifdef(CONFIG_ZTEST_SHELL src/ztest_shell.c)
zephyr_library_sources_ifdef(CONFIG_ZTRESS src/ztress.c)


Expand Down
52 changes: 52 additions & 0 deletions subsys/testsuite/ztest/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,58 @@ config TEST_LOGGING_FLUSH_AFTER_TEST
default y
depends on MULTITHREADING

config ZTEST_MAIN
Copy link
Member

Choose a reason for hiding this comment

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

why do we need this? Either you run with shell, then you jump into shell and run the tests from there, or run the tests without shell and just execute the test as usual. Do not see the value of this.

Copy link
Member Author

@cfriedt cfriedt Dec 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It is to prevent ZTest from running by default immediately. Some systems need additional application-time setup before you can start executing tests.

bool "Run ZTest test suites from main"
default y
help
When disabled, ZTest test suites are not automatically run from main().

config ZTEST_SHELL
bool "ZTest shell command"
select SHELL
select GETOPT
select FNMATCH
select GETOPT_LONG
select SHELL_GETOPT
# currently this is broken due to getopt_state / conflicting native definitions on POSIX arch
depends on !ARCH_POSIX
help
Provide a shell command to run ZTests from the Zephyr shell. The arguments of the 'ztest'
Copy link
Member

Choose a reason for hiding this comment

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

ztest and the zephyr shell can do more than what gtest does or provides, so this is rather limiting, i.e. I would like to see feature of the shell being used, such as sub commands, auto-completion etc instead of depending on command lines for everything.
Compatibility mode with gtest is fine, but not as the main shell implementation.

Copy link
Member Author

@cfriedt cfriedt Dec 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Subcommands are fine for a specific class of command. Here, flags make a bit more sense. Simply because subcommands exist is not justification to use them everywhere.

command are inspired by Google Test Framework.

if ZTEST_SHELL

config ZTEST_SHELL_PREFIX
string "Prefix for ztest shell options"
default ""
help
This option is for compatibility with Google Test Framework, where each option specific to
Copy link
Member

Choose a reason for hiding this comment

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

main motivation here seems to be compatibility with gtest instead of providing full shell support for tests, which is very limiting. And then, most of those options insipred by gtest are not implemented, so basically we go back what can be done already as demonstrated in #58374

Copy link
Member Author

@cfriedt cfriedt Dec 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

See earlier comment #65437 (comment)

Additional features were pretty trivial to implement, but require refactorization.

gtest is prefixed with 'gtest_' (e.g. '--gtest_shuffle'). This helps to separate options
specific to the test runner from application options.

config ZTEST_SHELL_NAME_SIZE_MAX
int "Max string length of suite.test concatenation"
default 64
help
Currently the should_test_run() method of struct ztest_arch_api separates the suite name
from the test name. The convention in gtest is to concatenate the two with a '.' separating
the suite name from the test name. Ensure that this option is large enough to concatenate
the largest suite.test string in desired testsuites.

Note: this buffer is placed on the stack.

config ZTEST_SHELL_PATTERN_MAX
int "Max string length of a ZTest filter"
default 32
help
The ztest shell filter option takes an argument of the form
POSITIVE_PATTERNS[-NEGATIVE_PATTERNS], where both the positive and negative components may
include several sub-patterns separated by ':'. Due to implementation details, it is
necessary to copy individual sub-patterns to a separate buffer before evaluation. This
option specifies the maximum size of that separate buffer.

endif

endif # ZTEST

config ZTEST_MOCKING
Expand Down
11 changes: 11 additions & 0 deletions subsys/testsuite/ztest/src/ztest.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,13 @@ static void end_report(void)
}
}

#ifdef CONFIG_ZTEST_SHELL
void __ztest_shell_end_report(void)
{
end_report();
}
#endif

static int cleanup_test(struct ztest_unit_test *test)
{
int ret = TC_PASS;
Expand Down Expand Up @@ -1072,6 +1079,8 @@ void ztest_verify_all_test_suites_ran(void)

void ztest_run_all(const void *state) { ztest_api.run_all(state); }

#ifdef CONFIG_ZTEST_MAIN

void __weak test_main(void)
{
ztest_run_all(NULL);
Expand Down Expand Up @@ -1156,3 +1165,5 @@ int main(void)
return 0;
}
#endif

#endif /* CONFIG_ZTEST_MAIN */
5 changes: 5 additions & 0 deletions subsys/testsuite/ztest/src/ztest_defaults.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ bool z_ztest_should_suite_run(const void *state, struct ztest_suite_node *suite)
return run_suite;
}

#ifdef CONFIG_ZTEST_SHELL
/* see ztest_shell.c */
bool z_ztest_should_test_run(const char *suite, const char *test);
#else
/**
* @brief Determines if the test case should run based on test cases listed
* in the command line argument. Run all tests for non-posix builds
Expand All @@ -66,6 +70,7 @@ bool z_ztest_should_test_run(const char *suite, const char *test)
{
return true;
}
#endif

ZTEST_DMEM const struct ztest_arch_api ztest_api = {
.run_all = z_ztest_run_all,
Expand Down
11 changes: 11 additions & 0 deletions subsys/testsuite/ztest/src/ztest_new.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,13 @@ static void end_report(void)
}
}

#ifdef CONFIG_ZTEST_SHELL
Copy link
Member

Choose a reason for hiding this comment

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

hmm, ztest_new.c should not be there, must have forgotten to remove it when old ztest was removed, so not sure about all those changes and why they were needed, strange.

Copy link
Member Author

@cfriedt cfriedt Dec 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I was just trying to be consistent. But yea was kind of surprised to see it still here. For anyone using tagged v3.5.0, it's still the active version for CONFIG_ZTEST_NEW

void __ztest_shell_end_report(void)
{
end_report();
}
#endif

static int cleanup_test(struct ztest_unit_test *test)
{
int ret = TC_PASS;
Expand Down Expand Up @@ -1072,6 +1079,8 @@ void ztest_verify_all_test_suites_ran(void)

void ztest_run_all(const void *state) { ztest_api.run_all(state); }

#ifdef CONFIG_ZTEST_MAIN

void __weak test_main(void)
{
ztest_run_all(NULL);
Expand Down Expand Up @@ -1156,3 +1165,5 @@ int main(void)
return 0;
}
#endif

#endif /* CONFIG_ZTEST_MAIN */
5 changes: 5 additions & 0 deletions subsys/testsuite/ztest/src/ztest_posix.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,10 @@ static bool z_ztest_testargs_contains(const char *suite_name, const char *test_n
return found;
}

#ifdef CONFIG_ZTEST_SHELL
/* see ztest_shell.c */
bool z_ztest_should_test_run(const char *suite, const char *test);
#else
/**
* @brief Determines if the test case should run based on test cases listed
* in the command line argument.
Expand All @@ -192,6 +196,7 @@ bool z_ztest_should_test_run(const char *suite, const char *test)

return run_test;
}
#endif

/**
* @brief Determines if the test suite should run based on test cases listed
Expand Down
Loading
Loading