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

Strict test ordering in new ztest API #41880

Closed
nordic-krch opened this issue Jan 17, 2022 · 6 comments
Closed

Strict test ordering in new ztest API #41880

nordic-krch opened this issue Jan 17, 2022 · 6 comments
Assignees
Labels
area: Test Framework Issues related not to a particular test, but to the framework instead Feature Request A request for a new feature

Comments

@nordic-krch
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Currently there are tests which rely on strict order execution. This is mainly related to user space test preparation where non-user test is executed before user test. In order to allow transition to new API it must be somehow supported. Example:

/* Do not disturb the ordering of these test cases */

Describe the solution you'd like
Strict ordering (adding line number to section name? Don't like it) or extend ZTEST_USER to accept some test specific setup_fn.

@nordic-krch nordic-krch added the Feature Request A request for a new feature label Jan 17, 2022
@nordic-krch nordic-krch added the area: Test Framework Issues related not to a particular test, but to the framework instead label Jan 17, 2022
@yperess
Copy link
Collaborator

yperess commented Jan 17, 2022

I'm working on a migration of the chromium tests that does this too. Here's how to set it up...

test_state.h

struct test_state {
  uint8_t phase;
};

main.c

void test_main(void)
{
  struct test_state state = {
    .phase = 0,
  };
  ztest_run_test_suites(&state);
  state.phase++;
  ztest_run_test_suites(&state);
  ztest_verify_all_test_suites_ran();
}

phase0.c

#include "test_state.h"
static bool phase0_predicate(const void *state)
{
  return ((struct test_state*)state)->phase == 0;
}

/* This will only run if state.phase == 0 */
ZTEST_SUITE(phase0, phase0_predicate, NULL, NULL, NULL, NULL);

ZTEST(phase0, ...

@yperess
Copy link
Collaborator

yperess commented Jan 17, 2022

Another thing if you don't want to make it one suite per set of tests is to combine them into one ZTEST that calls them in the right order. We do that in chromium. It makes more sense too since if one fails you want to stop execution anyway

@yperess
Copy link
Collaborator

yperess commented Feb 8, 2022

Can this issue be closed? Was the above enough to get you unblocked?

@nordic-krch
Copy link
Contributor Author

Frankly speaking, i am not convinced. I know that it may be cleaner but it is less intuitive and it's a regression (in terms of ease of use) compared to what we had. I'm afraid that if it is more cumbersome to setup more complex test scenario user will simple skip adding test like that.

If we take example mention in the issue we had following sequence:

	ztest_test_suite(workqueue_api,
			 /* Do not disturb the ordering of these test cases */
			 ztest_user_unit_test(test_work_user_queue_start_before_submit),
			 ztest_unit_test(test_user_workq_granted_access_setup),
			 ztest_user_unit_test(test_user_workq_granted_access),

So it was quite obvious, we have ordering, some tests are in user space other not. Comparing that to using predicate, implementing weak test_main function where you need to know what to call there. It's much more complex.

@yperess
Copy link
Collaborator

yperess commented Jun 30, 2022

I'm sorry, I can't say that this is a regression. By definition unit tests should NEVER depend on the order. We've even added a shuffle feature for developers to be able to find these kinds of conditions and label tests as flaky. A strict dependency on the order means that it's actually one test. @aaronemassey can you discuss this in the testing working group tomorrow?

@nordic-krch
Copy link
Contributor Author

I agree that it's not good approach. It's just that legacy ztest had that and it was used. Mainly for userspace test setup. It would be good to provide user friendly alternative. The proposal above is not that intuitive but if nothing better is proposed than it's ok for me to close it.

@yperess yperess closed this as completed Jul 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Test Framework Issues related not to a particular test, but to the framework instead Feature Request A request for a new feature
Projects
None yet
Development

No branches or pull requests

2 participants