| Message ID | 20190306152430.25219-1-chrubis@suse.cz |
|---|---|
| State | Superseded |
| Delegated to: | Petr Vorel |
| Headers | show |
| Series | [RFC,1/2] tst_test: Add test multiplex function | expand |
----- Original Message ----- > The test multiplex function is intended for running the test function in > a loop for a different settings. The indended purpose is to run tests > for both libc wrapper and raw syscall as well as for different syscall > variants. > > The commit itself adds a test_multiplex() function into the tst_test > structure, if set this function is called in a loop before each test > iteration and is responsible for changing the test variant into next > one. The iterations continue until the function returns zero. Hi, on first look this looks like a workaround, because we have locked ourselves out of .test function for timer tests. Is there some way we could allow .test again for times tests? Something user could override, provided he then runs the library function that iterates over all usec values. (sorry for pseudo-code) // test void my_test(int n) { select_func = n; test_timer_test(); } // library void test_timer_test() { for (i = 0; i < ARRAY_SIZE(tcases); i++) timer_test_fn(i); } Regards, Jan > > Signed-off-by: Cyril Hrubis <chrubis@suse.cz> > CC: Mark Salyzyn <salyzyn@android.com> > CC: Steve Muckle <smuckle@google.com> > CC: Jan Stancek <jstancek@redhat.com> > --- > include/tst_test.h | 14 ++++++++++++++ > lib/tst_test.c | 11 +++++++++-- > 2 files changed, 23 insertions(+), 2 deletions(-) > > diff --git a/include/tst_test.h b/include/tst_test.h > index da41f776d..b747b24d5 100644 > --- a/include/tst_test.h > +++ b/include/tst_test.h > @@ -146,6 +146,20 @@ struct tst_test { > */ > int all_filesystems:1; > > + /* > + * If set this function is used to mutiplex between different test > + * variants. > + * > + * Multiplexers are the way how to run the same test for different > + * settings. The intended use is to test different syscall > + * wrappers/variants but the API is generic and does not limit the > + * usage in any way. > + * > + * Each time the function is called it switches to next test variant, > + * returns zero if all variants has been iterated over. > + */ > + int (*test_multiplex)(void); > + > /* Minimal device size in megabytes */ > unsigned int dev_min_size; > > diff --git a/lib/tst_test.c b/lib/tst_test.c > index 7dd890b8d..6b033c879 100644 > --- a/lib/tst_test.c > +++ b/lib/tst_test.c > @@ -1009,8 +1009,15 @@ static void testrun(void) > if (!cont) > break; > > - run_tests(); > - heartbeat(); > + if (tst_test->test_multiplex) { > + while (tst_test->test_multiplex()) { > + run_tests(); > + heartbeat(); > + } > + } else { > + run_tests(); > + heartbeat(); > + } > } > > do_test_cleanup(); > -- > 2.19.2 > >
Hi! > on first look this looks like a workaround, because we have locked > ourselves out of .test function for timer tests. I do not follow you here, can you elaborate? This patch has nothing to do with timer tests, it just allows whatever the test does to be done several times with a hook to change some settings prior to each iteration.
----- Original Message ----- > Hi! > > on first look this looks like a workaround, because we have locked > > ourselves out of .test function for timer tests. > > I do not follow you here, can you elaborate? If this wasn't timer test, I'd ask why don't we use existing .test and .tcnt, your test() func can be called with a parameter, so you could change the code to choose correct syscall/glibc func based on value of that parameter. For normal tests, this looks almost and .test/.tcnt functionality, except test count can be also dynamic. static int tcase = -1; static void test(void) { switch (tcase) { } } static int select_mpx(void) { tcase++; if (tcase == 5) return 0; return 1; } static struct tst_test test = { .test_multiplex = select_mpx, .test_all = test, } > > This patch has nothing to do with timer tests, it just allows whatever > the test does to be done several times with a hook to change some > settings prior to each iteration. > > -- > Cyril Hrubis > chrubis@suse.cz >
Hi! > > > on first look this looks like a workaround, because we have locked > > > ourselves out of .test function for timer tests. > > > > I do not follow you here, can you elaborate? > > If this wasn't timer test, I'd ask why don't we use existing .test and .tcnt, > your test() func can be called with a parameter, so you could change > the code to choose correct syscall/glibc func based on value of that parameter. > > For normal tests, this looks almost and .test/.tcnt functionality, > except test count can be also dynamic. Well so does the .all_filesystems flag, however I see these concepts to be orthogonal to what the actual test does, which is the reason I want them hooked up in the library rather than to be part of the testcase code.
----- Original Message ----- > Hi! > > > > on first look this looks like a workaround, because we have locked > > > > ourselves out of .test function for timer tests. > > > > > > I do not follow you here, can you elaborate? > > > > If this wasn't timer test, I'd ask why don't we use existing .test and > > .tcnt, > > your test() func can be called with a parameter, so you could change > > the code to choose correct syscall/glibc func based on value of that > > parameter. > > > > For normal tests, this looks almost and .test/.tcnt functionality, > > except test count can be also dynamic. > > Well so does the .all_filesystems flag, however I see these concepts to > be orthogonal to what the actual test does, which is the reason I want > them hooked up in the library rather than to be part of the testcase > code. OK, so it's like another level on top of current test functions. I probably would like 'variant' somewhere in name more, but overall I'm not against the patch. Are you planning on adding some docs too? Regards, Jan
On 03/06/2019 07:24 AM, Cyril Hrubis wrote: > The test multiplex function is intended for running the test function in > a loop for a different settings. The indended purpose is to run tests > for both libc wrapper and raw syscall as well as for different syscall > variants. > > The commit itself adds a test_multiplex() function into the tst_test > structure, if set this function is called in a loop before each test > iteration and is responsible for changing the test variant into next > one. The iterations continue until the function returns zero. > > Signed-off-by: Cyril Hrubis <chrubis@suse.cz> > CC: Mark Salyzyn <salyzyn@android.com> > CC: Steve Muckle <smuckle@google.com> > CC: Jan Stancek <jstancek@redhat.com> Approach seems good to me. Reviewed-by: Steve Muckle <smuckle@google.com> > --- > include/tst_test.h | 14 ++++++++++++++ > lib/tst_test.c | 11 +++++++++-- > 2 files changed, 23 insertions(+), 2 deletions(-) > > diff --git a/include/tst_test.h b/include/tst_test.h > index da41f776d..b747b24d5 100644 > --- a/include/tst_test.h > +++ b/include/tst_test.h > @@ -146,6 +146,20 @@ struct tst_test { > */ > int all_filesystems:1; > > + /* > + * If set this function is used to mutiplex between different test > + * variants. > + * > + * Multiplexers are the way how to run the same test for different > + * settings. The intended use is to test different syscall > + * wrappers/variants but the API is generic and does not limit the > + * usage in any way. > + * > + * Each time the function is called it switches to next test variant, > + * returns zero if all variants has been iterated over. > + */ > + int (*test_multiplex)(void); > + > /* Minimal device size in megabytes */ > unsigned int dev_min_size; > > diff --git a/lib/tst_test.c b/lib/tst_test.c > index 7dd890b8d..6b033c879 100644 > --- a/lib/tst_test.c > +++ b/lib/tst_test.c > @@ -1009,8 +1009,15 @@ static void testrun(void) > if (!cont) > break; > > - run_tests(); > - heartbeat(); > + if (tst_test->test_multiplex) { > + while (tst_test->test_multiplex()) { > + run_tests(); > + heartbeat(); > + } > + } else { > + run_tests(); > + heartbeat(); > + } > } > > do_test_cleanup(); >
Hi! > > > If this wasn't timer test, I'd ask why don't we use existing .test and > > > .tcnt, > > > your test() func can be called with a parameter, so you could change > > > the code to choose correct syscall/glibc func based on value of that > > > parameter. > > > > > > For normal tests, this looks almost and .test/.tcnt functionality, > > > except test count can be also dynamic. > > > > Well so does the .all_filesystems flag, however I see these concepts to > > be orthogonal to what the actual test does, which is the reason I want > > them hooked up in the library rather than to be part of the testcase > > code. > > OK, so it's like another level on top of current test functions. Yes, the change to the library adds generic layer for that. > I probably would like 'variant' somewhere in name more, but overall > I'm not against the patch. Are you planning on adding some docs too? I thought that calling it multiplex was descriptive enough name. There is some summary in: https://github.com/linux-test-project/ltp/issues/506 I guess that I can expand the text a bit and put it in the doc/ directory in the source tree along with the change to the test library.
diff --git a/include/tst_test.h b/include/tst_test.h index da41f776d..b747b24d5 100644 --- a/include/tst_test.h +++ b/include/tst_test.h @@ -146,6 +146,20 @@ struct tst_test { */ int all_filesystems:1; + /* + * If set this function is used to mutiplex between different test + * variants. + * + * Multiplexers are the way how to run the same test for different + * settings. The intended use is to test different syscall + * wrappers/variants but the API is generic and does not limit the + * usage in any way. + * + * Each time the function is called it switches to next test variant, + * returns zero if all variants has been iterated over. + */ + int (*test_multiplex)(void); + /* Minimal device size in megabytes */ unsigned int dev_min_size; diff --git a/lib/tst_test.c b/lib/tst_test.c index 7dd890b8d..6b033c879 100644 --- a/lib/tst_test.c +++ b/lib/tst_test.c @@ -1009,8 +1009,15 @@ static void testrun(void) if (!cont) break; - run_tests(); - heartbeat(); + if (tst_test->test_multiplex) { + while (tst_test->test_multiplex()) { + run_tests(); + heartbeat(); + } + } else { + run_tests(); + heartbeat(); + } } do_test_cleanup();
The test multiplex function is intended for running the test function in a loop for a different settings. The indended purpose is to run tests for both libc wrapper and raw syscall as well as for different syscall variants. The commit itself adds a test_multiplex() function into the tst_test structure, if set this function is called in a loop before each test iteration and is responsible for changing the test variant into next one. The iterations continue until the function returns zero. Signed-off-by: Cyril Hrubis <chrubis@suse.cz> CC: Mark Salyzyn <salyzyn@android.com> CC: Steve Muckle <smuckle@google.com> CC: Jan Stancek <jstancek@redhat.com> --- include/tst_test.h | 14 ++++++++++++++ lib/tst_test.c | 11 +++++++++-- 2 files changed, 23 insertions(+), 2 deletions(-)