Attention is currently required from: Aarya, Anastasia Klimchuk, Edward O'Callaghan, Nikolai Artemiev, Simon Buhrow, Thomas Heijligen.

View Change

2 comments:

• File tests/erase_func_algo.c:

  • Patch Set #3, Line 238: void **state

    I found something in cmocka docs, around this page https://api.cmocka.org/group__cmocka__exec. […]

    I think it's worth doing that, but you don't need to expose much detail.

    If you call `_cmocka_run_group_tests`, then you need to be given a list of test cases which can be allocated in any way desired. It would also be useful to make each test case in `g_state` its own actual test case so it's easier to read the results rather than lumping a bunch of test cases into one test.

    To that end, I imagine providing a function that returns a bunch of test cases:

    ```
    static const struct test_case {
      // ...
    } test_cases[] = {
      // ...
    };

    struct test_state {
      uint8_t chip_contents[1024];
      const struct test_case *test_case;
    };

    static int setup(void **state) {
      const struct test_case *test_case = *state;
      
      *state = malloc(sizeof(struct test_state));
      state->test_case = test_case;
      // Initialize other test state as desired
      return 0;
    }

    static int teardown(void **state) {
      free(*state);
      return 0;
    }

    const CMUnitTest *get_erase_func_algo_tests(size_t *num_tests) {
      CMUnitTest *out = calloc(ARRAY_SIZE(test_cases), sizeof(*test_cases));
      for (size_t i = 0; i < ARRAY_SIZE(test_cases); i++) {
        out[i] = (CMUnitTest){
          .setup_func = setup,
          .teardown_func = teardown,
          .initial_state = &test_cases[i],
          .test_func = run_erase_and_verify,
        };
      }
      
      *num_tests = ARRAY_SIZE(test_cases);
      return out;
    }
    ```

    Then in main run those test cases:

    ```
    size_t n_erase_tests;
    const CMUnitTest *erase_tests = get_erase_func_algo_tests(&n_erase_tests);
    _cmocka_run_group_tests("erase stuff", erase_tests, n_erase_tests, NULL, NULL);
    ```

• File tests/erase_func_algo.c:

  • Patch Set #14, Line 61: struct test_case test_cases[TEST_CASES_NUM]; /* Data for running tests. */

    I'd prefer to make this a `const struct test_case *` just to capture that the test case data is immutable, but then it seems even be better to replace `ind` with a pointer to a const `test_case` so functions that receive test state don't have access to irrelevant data (and the only place the table of test cases needs to be wrangled is in the functions that iterate over them to generate each test case).

To view, visit change 67535. To unsubscribe, or for help writing mail filters, visit settings.