| Message ID | 20240424075955.3604997-11-shinichiro.kawasaki@wdc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | support test case repeat by different conditions | expand |
On 24/04/2024 10:59, Shin'ichiro Kawasaki wrote: > Enable repeated test runs for the listed test cases for > NVMET_BLKDEV_TYPES. Modify the set_conditions() hooks to call > _set_nvme_trtype_and_nvmet_blkdev_type() instead of _set_nvmet_trtype() > so that the test cases are repeated for listed conditions in > NVMET_BLKDEV_TYPES and NVMET_TRTYPES. > > The default values of NVMET_BLKDEV_TYPES is (device file). With this > default set up, each of the listed test cases are run twice. The second > runs of the test cases for 'file' blkdev type do exact same test as > other test cases nvme/007, 009, 011, 013, 015, 020 and 024. > > Reviewed-by: Daniel Wagner <dwagner@suse.de> > Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com> > Acked-by: Nitesh Shetty <nj.shetty@samsung.com> > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> > --- > tests/nvme/006 | 2 +- > tests/nvme/008 | 2 +- > tests/nvme/010 | 2 +- > tests/nvme/012 | 2 +- > tests/nvme/014 | 2 +- > tests/nvme/019 | 2 +- > tests/nvme/023 | 2 +- > 7 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/tests/nvme/006 b/tests/nvme/006 > index ff0a9eb..c543b40 100755 > --- a/tests/nvme/006 > +++ b/tests/nvme/006 > @@ -16,7 +16,7 @@ requires() { > } > > set_conditions() { > - _set_nvme_trtype "$@" > + _set_nvme_trtype_and_nvmet_blkdev_type "$@" Why not calling separate functions? having func do_a_and_b interface is not great.
On Apr 28, 2024 / 11:58, Sagi Grimberg wrote: > > > On 24/04/2024 10:59, Shin'ichiro Kawasaki wrote: > > Enable repeated test runs for the listed test cases for > > NVMET_BLKDEV_TYPES. Modify the set_conditions() hooks to call > > _set_nvme_trtype_and_nvmet_blkdev_type() instead of _set_nvmet_trtype() > > so that the test cases are repeated for listed conditions in > > NVMET_BLKDEV_TYPES and NVMET_TRTYPES. > > > > The default values of NVMET_BLKDEV_TYPES is (device file). With this > > default set up, each of the listed test cases are run twice. The second > > runs of the test cases for 'file' blkdev type do exact same test as > > other test cases nvme/007, 009, 011, 013, 015, 020 and 024. > > > > Reviewed-by: Daniel Wagner <dwagner@suse.de> > > Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com> > > Acked-by: Nitesh Shetty <nj.shetty@samsung.com> > > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> > > --- > > tests/nvme/006 | 2 +- > > tests/nvme/008 | 2 +- > > tests/nvme/010 | 2 +- > > tests/nvme/012 | 2 +- > > tests/nvme/014 | 2 +- > > tests/nvme/019 | 2 +- > > tests/nvme/023 | 2 +- > > 7 files changed, 7 insertions(+), 7 deletions(-) > > > > diff --git a/tests/nvme/006 b/tests/nvme/006 > > index ff0a9eb..c543b40 100755 > > --- a/tests/nvme/006 > > +++ b/tests/nvme/006 > > @@ -16,7 +16,7 @@ requires() { > > } > > set_conditions() { > > - _set_nvme_trtype "$@" > > + _set_nvme_trtype_and_nvmet_blkdev_type "$@" > > Why not calling separate functions? having func do_a_and_b interface is not > great. In this case, we want to repeat the test cases to cover combination of two conditions: M trtypes and N blkdev_types. The test case should be repeated to cover all of M x N matrix elements, then the hook set_conditions() should iterate the elements. I can not think of the way to handle this iteration with separated two functions.
On 28/04/2024 13:32, Shinichiro Kawasaki wrote: > On Apr 28, 2024 / 11:58, Sagi Grimberg wrote: >> >> On 24/04/2024 10:59, Shin'ichiro Kawasaki wrote: >>> Enable repeated test runs for the listed test cases for >>> NVMET_BLKDEV_TYPES. Modify the set_conditions() hooks to call >>> _set_nvme_trtype_and_nvmet_blkdev_type() instead of _set_nvmet_trtype() >>> so that the test cases are repeated for listed conditions in >>> NVMET_BLKDEV_TYPES and NVMET_TRTYPES. >>> >>> The default values of NVMET_BLKDEV_TYPES is (device file). With this >>> default set up, each of the listed test cases are run twice. The second >>> runs of the test cases for 'file' blkdev type do exact same test as >>> other test cases nvme/007, 009, 011, 013, 015, 020 and 024. >>> >>> Reviewed-by: Daniel Wagner <dwagner@suse.de> >>> Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com> >>> Acked-by: Nitesh Shetty <nj.shetty@samsung.com> >>> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> >>> --- >>> tests/nvme/006 | 2 +- >>> tests/nvme/008 | 2 +- >>> tests/nvme/010 | 2 +- >>> tests/nvme/012 | 2 +- >>> tests/nvme/014 | 2 +- >>> tests/nvme/019 | 2 +- >>> tests/nvme/023 | 2 +- >>> 7 files changed, 7 insertions(+), 7 deletions(-) >>> >>> diff --git a/tests/nvme/006 b/tests/nvme/006 >>> index ff0a9eb..c543b40 100755 >>> --- a/tests/nvme/006 >>> +++ b/tests/nvme/006 >>> @@ -16,7 +16,7 @@ requires() { >>> } >>> set_conditions() { >>> - _set_nvme_trtype "$@" >>> + _set_nvme_trtype_and_nvmet_blkdev_type "$@" >> Why not calling separate functions? having func do_a_and_b interface is not >> great. > In this case, we want to repeat the test cases to cover combination of two > conditions: M trtypes and N blkdev_types. The test case should be repeated to > cover all of M x N matrix elements, then the hook set_conditions() should > iterate the elements. I can not think of the way to handle this iteration with > separated two functions. What happens when you add another condition to iterate against, you introduce set_a_and_b_and_c interface?
On Apr 28, 2024 / 16:12, Sagi Grimberg wrote: > > > On 28/04/2024 13:32, Shinichiro Kawasaki wrote: > > On Apr 28, 2024 / 11:58, Sagi Grimberg wrote: > > > > > > On 24/04/2024 10:59, Shin'ichiro Kawasaki wrote: > > > > Enable repeated test runs for the listed test cases for > > > > NVMET_BLKDEV_TYPES. Modify the set_conditions() hooks to call > > > > _set_nvme_trtype_and_nvmet_blkdev_type() instead of _set_nvmet_trtype() > > > > so that the test cases are repeated for listed conditions in > > > > NVMET_BLKDEV_TYPES and NVMET_TRTYPES. > > > > > > > > The default values of NVMET_BLKDEV_TYPES is (device file). With this > > > > default set up, each of the listed test cases are run twice. The second > > > > runs of the test cases for 'file' blkdev type do exact same test as > > > > other test cases nvme/007, 009, 011, 013, 015, 020 and 024. > > > > > > > > Reviewed-by: Daniel Wagner <dwagner@suse.de> > > > > Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com> > > > > Acked-by: Nitesh Shetty <nj.shetty@samsung.com> > > > > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> > > > > --- > > > > tests/nvme/006 | 2 +- > > > > tests/nvme/008 | 2 +- > > > > tests/nvme/010 | 2 +- > > > > tests/nvme/012 | 2 +- > > > > tests/nvme/014 | 2 +- > > > > tests/nvme/019 | 2 +- > > > > tests/nvme/023 | 2 +- > > > > 7 files changed, 7 insertions(+), 7 deletions(-) > > > > > > > > diff --git a/tests/nvme/006 b/tests/nvme/006 > > > > index ff0a9eb..c543b40 100755 > > > > --- a/tests/nvme/006 > > > > +++ b/tests/nvme/006 > > > > @@ -16,7 +16,7 @@ requires() { > > > > } > > > > set_conditions() { > > > > - _set_nvme_trtype "$@" > > > > + _set_nvme_trtype_and_nvmet_blkdev_type "$@" > > > Why not calling separate functions? having func do_a_and_b interface is not > > > great. > > In this case, we want to repeat the test cases to cover combination of two > > conditions: M trtypes and N blkdev_types. The test case should be repeated to > > cover all of M x N matrix elements, then the hook set_conditions() should > > iterate the elements. I can not think of the way to handle this iteration with > > separated two functions. > > What happens when you add another condition to iterate against, you > introduce set_a_and_b_and_c interface? That is my current intent. Another question is how it is likely to have more conditions to add on. I guess such many, multiplied conditions will result in combination explosion and long test runtime, so I'm not sure how much it will be useful. Do we have potential candidates of the third or fourth conditions?
On 29/04/2024 2:58, Shinichiro Kawasaki wrote: > On Apr 28, 2024 / 16:12, Sagi Grimberg wrote: >> >> On 28/04/2024 13:32, Shinichiro Kawasaki wrote: >>> On Apr 28, 2024 / 11:58, Sagi Grimberg wrote: >>>> On 24/04/2024 10:59, Shin'ichiro Kawasaki wrote: >>>>> Enable repeated test runs for the listed test cases for >>>>> NVMET_BLKDEV_TYPES. Modify the set_conditions() hooks to call >>>>> _set_nvme_trtype_and_nvmet_blkdev_type() instead of _set_nvmet_trtype() >>>>> so that the test cases are repeated for listed conditions in >>>>> NVMET_BLKDEV_TYPES and NVMET_TRTYPES. >>>>> >>>>> The default values of NVMET_BLKDEV_TYPES is (device file). With this >>>>> default set up, each of the listed test cases are run twice. The second >>>>> runs of the test cases for 'file' blkdev type do exact same test as >>>>> other test cases nvme/007, 009, 011, 013, 015, 020 and 024. >>>>> >>>>> Reviewed-by: Daniel Wagner <dwagner@suse.de> >>>>> Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com> >>>>> Acked-by: Nitesh Shetty <nj.shetty@samsung.com> >>>>> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> >>>>> --- >>>>> tests/nvme/006 | 2 +- >>>>> tests/nvme/008 | 2 +- >>>>> tests/nvme/010 | 2 +- >>>>> tests/nvme/012 | 2 +- >>>>> tests/nvme/014 | 2 +- >>>>> tests/nvme/019 | 2 +- >>>>> tests/nvme/023 | 2 +- >>>>> 7 files changed, 7 insertions(+), 7 deletions(-) >>>>> >>>>> diff --git a/tests/nvme/006 b/tests/nvme/006 >>>>> index ff0a9eb..c543b40 100755 >>>>> --- a/tests/nvme/006 >>>>> +++ b/tests/nvme/006 >>>>> @@ -16,7 +16,7 @@ requires() { >>>>> } >>>>> set_conditions() { >>>>> - _set_nvme_trtype "$@" >>>>> + _set_nvme_trtype_and_nvmet_blkdev_type "$@" >>>> Why not calling separate functions? having func do_a_and_b interface is not >>>> great. >>> In this case, we want to repeat the test cases to cover combination of two >>> conditions: M trtypes and N blkdev_types. The test case should be repeated to >>> cover all of M x N matrix elements, then the hook set_conditions() should >>> iterate the elements. I can not think of the way to handle this iteration with >>> separated two functions. >> What happens when you add another condition to iterate against, you >> introduce set_a_and_b_and_c interface? > That is my current intent. I don't think its very maintainable. > > Another question is how it is likely to have more conditions to add on. I expect that people will want to add more flavors moving forward. For example ADDRESS_FAMILIES="ipv4 ipv6" RDMA_TRANSPORT="siw rxe" and possibly other features that can grow in the future. > I guess > such many, multiplied conditions will result in combination explosion and long > test runtime, so I'm not sure how much it will be useful. I think that running multiple flavors of a test suite is a capability that is bound to be reused as more test flavors emerge. But that may be just my opinion. > > Do we have potential candidates of the third or fourth conditions? Yes, see above.
On Apr 30, 2024 / 15:00, Sagi Grimberg wrote: > > > On 29/04/2024 2:58, Shinichiro Kawasaki wrote: > > On Apr 28, 2024 / 16:12, Sagi Grimberg wrote: > > > > > > On 28/04/2024 13:32, Shinichiro Kawasaki wrote: > > > > On Apr 28, 2024 / 11:58, Sagi Grimberg wrote: > > > > > On 24/04/2024 10:59, Shin'ichiro Kawasaki wrote: > > > > > > Enable repeated test runs for the listed test cases for > > > > > > NVMET_BLKDEV_TYPES. Modify the set_conditions() hooks to call > > > > > > _set_nvme_trtype_and_nvmet_blkdev_type() instead of _set_nvmet_trtype() > > > > > > so that the test cases are repeated for listed conditions in > > > > > > NVMET_BLKDEV_TYPES and NVMET_TRTYPES. > > > > > > > > > > > > The default values of NVMET_BLKDEV_TYPES is (device file). With this > > > > > > default set up, each of the listed test cases are run twice. The second > > > > > > runs of the test cases for 'file' blkdev type do exact same test as > > > > > > other test cases nvme/007, 009, 011, 013, 015, 020 and 024. > > > > > > > > > > > > Reviewed-by: Daniel Wagner <dwagner@suse.de> > > > > > > Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com> > > > > > > Acked-by: Nitesh Shetty <nj.shetty@samsung.com> > > > > > > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> > > > > > > --- > > > > > > tests/nvme/006 | 2 +- > > > > > > tests/nvme/008 | 2 +- > > > > > > tests/nvme/010 | 2 +- > > > > > > tests/nvme/012 | 2 +- > > > > > > tests/nvme/014 | 2 +- > > > > > > tests/nvme/019 | 2 +- > > > > > > tests/nvme/023 | 2 +- > > > > > > 7 files changed, 7 insertions(+), 7 deletions(-) > > > > > > > > > > > > diff --git a/tests/nvme/006 b/tests/nvme/006 > > > > > > index ff0a9eb..c543b40 100755 > > > > > > --- a/tests/nvme/006 > > > > > > +++ b/tests/nvme/006 > > > > > > @@ -16,7 +16,7 @@ requires() { > > > > > > } > > > > > > set_conditions() { > > > > > > - _set_nvme_trtype "$@" > > > > > > + _set_nvme_trtype_and_nvmet_blkdev_type "$@" > > > > > Why not calling separate functions? having func do_a_and_b interface is not > > > > > great. > > > > In this case, we want to repeat the test cases to cover combination of two > > > > conditions: M trtypes and N blkdev_types. The test case should be repeated to > > > > cover all of M x N matrix elements, then the hook set_conditions() should > > > > iterate the elements. I can not think of the way to handle this iteration with > > > > separated two functions. > > > What happens when you add another condition to iterate against, you > > > introduce set_a_and_b_and_c interface? > > That is my current intent. > > I don't think its very maintainable. > > > > > Another question is how it is likely to have more conditions to add on. > > I expect that people will want to add more flavors moving forward. For > example > ADDRESS_FAMILIES="ipv4 ipv6" RDMA_TRANSPORT="siw rxe" and possibly other > features that can grow in the future. > > > > I guess > > such many, multiplied conditions will result in combination explosion and long > > test runtime, so I'm not sure how much it will be useful. > > I think that running multiple flavors of a test suite is a capability that > is bound to be > reused as more test flavors emerge. But that may be just my opinion. > > > > > Do we have potential candidates of the third or fourth conditions? > > Yes, see above. Okay, the candidates look useful in the future. Fortunately, I came up with an idea to make up the condition matrix from multiple set_conditions() hooks. Will try to implement it and respin the series.
>>> Another question is how it is likely to have more conditions to add on. >> I expect that people will want to add more flavors moving forward. For >> example >> ADDRESS_FAMILIES="ipv4 ipv6" RDMA_TRANSPORT="siw rxe" and possibly other >> features that can grow in the future. >> >> >>> I guess >>> such many, multiplied conditions will result in combination explosion and long >>> test runtime, so I'm not sure how much it will be useful. >> I think that running multiple flavors of a test suite is a capability that >> is bound to be >> reused as more test flavors emerge. But that may be just my opinion. >> >>> Do we have potential candidates of the third or fourth conditions? >> Yes, see above. > Okay, the candidates look useful in the future. Fortunately, I came up with an > idea to make up the condition matrix from multiple set_conditions() hooks. Will > try to implement it and respin the series. Cool.
diff --git a/tests/nvme/006 b/tests/nvme/006 index ff0a9eb..c543b40 100755 --- a/tests/nvme/006 +++ b/tests/nvme/006 @@ -16,7 +16,7 @@ requires() { } set_conditions() { - _set_nvme_trtype "$@" + _set_nvme_trtype_and_nvmet_blkdev_type "$@" } test() { diff --git a/tests/nvme/008 b/tests/nvme/008 index 1877d8a..b53ecdb 100755 --- a/tests/nvme/008 +++ b/tests/nvme/008 @@ -16,7 +16,7 @@ requires() { } set_conditions() { - _set_nvme_trtype "$@" + _set_nvme_trtype_and_nvmet_blkdev_type "$@" } test() { diff --git a/tests/nvme/010 b/tests/nvme/010 index 34914a7..0417daf 100755 --- a/tests/nvme/010 +++ b/tests/nvme/010 @@ -16,7 +16,7 @@ requires() { } set_conditions() { - _set_nvme_trtype "$@" + _set_nvme_trtype_and_nvmet_blkdev_type "$@" } test() { diff --git a/tests/nvme/012 b/tests/nvme/012 index e06bf8d..37b9056 100755 --- a/tests/nvme/012 +++ b/tests/nvme/012 @@ -20,7 +20,7 @@ requires() { } set_conditions() { - _set_nvme_trtype "$@" + _set_nvme_trtype_and_nvmet_blkdev_type "$@" } test() { diff --git a/tests/nvme/014 b/tests/nvme/014 index ff0ebfb..bcfbc87 100755 --- a/tests/nvme/014 +++ b/tests/nvme/014 @@ -16,7 +16,7 @@ requires() { } set_conditions() { - _set_nvme_trtype "$@" + _set_nvme_trtype_and_nvmet_blkdev_type "$@" } test() { diff --git a/tests/nvme/019 b/tests/nvme/019 index 31020d9..fb11d41 100755 --- a/tests/nvme/019 +++ b/tests/nvme/019 @@ -16,7 +16,7 @@ requires() { } set_conditions() { - _set_nvme_trtype "$@" + _set_nvme_trtype_and_nvmet_blkdev_type "$@" } test() { diff --git a/tests/nvme/023 b/tests/nvme/023 index da99406..a723b73 100755 --- a/tests/nvme/023 +++ b/tests/nvme/023 @@ -16,7 +16,7 @@ requires() { } set_conditions() { - _set_nvme_trtype "$@" + _set_nvme_trtype_and_nvmet_blkdev_type "$@" } test() {