On Fri, Oct 8, 2021 at 4:06 PM sunyucong@xxxxxxxxx <sunyucong@xxxxxxxxx> wrote:
>
> On Fri, Oct 8, 2021 at 3:26 PM Andrii Nakryiko
> <andrii.nakryiko@xxxxxxxxx> wrote:
> >
> > On Wed, Oct 6, 2021 at 11:56 AM Yucong Sun <fallentree@xxxxxx> wrote:
> > >
> > > From: Yucong Sun <sunyucong@xxxxxxxxx>
> > >
> > > This patch allows tests to define serial_test_name() instead of
> > > test_name(), and this will make test_progs execute those in sequence
> > > after all other tests finished executing concurrently.
> > >
> > > Signed-off-by: Yucong Sun <sunyucong@xxxxxxxxx>
> > > ---
> > > tools/testing/selftests/bpf/test_progs.c | 60 +++++++++++++++++++++---
> > > 1 file changed, 54 insertions(+), 6 deletions(-)
> > >
> >
> > [...]
> >
> > > @@ -1129,6 +1136,40 @@ static int server_main(void)
> > > free(env.worker_current_test);
> > > free(data);
> > >
> > > + /* run serial tests */
> > > + save_netns();
> > > +
> > > + for (int i = 0; i < prog_test_cnt; i++) {
> > > + struct prog_test_def *test = &prog_test_defs[i];
> > > + struct test_result *result = &test_results[i];
> > > +
> > > + if (!test->should_run || !test->run_serial_test)
> > > + continue;
> > > +
> > > + stdio_hijack();
> > > +
> > > + run_one_test(i);
> > > +
> > > + stdio_restore();
> > > + if (env.log_buf) {
> > > + result->log_cnt = env.log_cnt;
> > > + result->log_buf = strdup(env.log_buf);
> > > +
> > > + free(env.log_buf);
> > > + env.log_buf = NULL;
> > > + env.log_cnt = 0;
> > > + }
> > > + restore_netns();
> > > +
> > > + fprintf(stdout, "#%d %s:%s\n",
> > > + test->test_num, test->test_name,
> > > + test->error_cnt ? "FAIL" : (test->skip_cnt ? "SKIP" : "OK"));
> > > +
> > > + result->error_cnt = test->error_cnt;
> > > + result->skip_cnt = test->skip_cnt;
> > > + result->sub_succ_cnt = test->sub_succ_cnt;
> > > + }
> > > +
> >
> > Did you try to just reuse sequential running loop logic in main() for
> > this? I'd like to avoid the third test running loop copy, if possible.
> > What were the problems of reusing the sequential logic from main(),
> > they do the same work, no?
>
> Well, yes and no
>
> The loop itself is small/simple enough, I'm not sure there is a value
> to extract them to a common function with multiple arguments.
The loop has netns save/restore, stdio hijacking, output formatting,
we might add some more logic later. I'm mainly asking because there is
already a sequential loop in the main, and I was wondering if we can
reuse that (as in, let it run regardless of -j option, just run only
serial tests if -j is specified).
> I think the main issue that needs to be refactored is that log
> printing still works differently in serial mode or parallel mode, it
> works now, but I would like to get rid of the old dump_test_log()
> function.
>
> >
> >
> > > /* generate summary */
> > > fflush(stderr);
> > > fflush(stdout);
> > > @@ -1326,6 +1367,13 @@ int main(int argc, char **argv)
> > > test->should_run = true;
> > > else
> > > test->should_run = false;
> > > +
> > > + if ((test->run_test == NULL && test->run_serial_test == NULL) ||
> > > + (test->run_test != NULL && test->run_serial_test != NULL)) {
> > > + fprintf(stderr, "Test %d:%s must have either test_%s() or serial_test_%sl() defined.\n",
> > > + test->test_num, test->test_name, test->test_name, test->test_name);
> > > + exit(EXIT_ERR_SETUP_INFRA);
> > > + }
> > > }
> > >
> > > /* ignore workers if we are just listing */
> > > --
> > > 2.30.2
> > >
