diff mbox series

[blktests,v3,10/15] nvme/{006,008,010,012,014,019,023}: support NVMET_BLKDEV_TYPES

Message ID 20240424075955.3604997-11-shinichiro.kawasaki@wdc.com (mailing list archive)
State New, archived
Headers show
Series support test case repeat by different conditions | expand

Commit Message

Shinichiro Kawasaki April 24, 2024, 7:59 a.m. UTC
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(-)

Comments

Sagi Grimberg April 28, 2024, 8:58 a.m. UTC | #1
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.
Shinichiro Kawasaki April 28, 2024, 10:32 a.m. UTC | #2
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.
Sagi Grimberg April 28, 2024, 1:12 p.m. UTC | #3
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?
Shinichiro Kawasaki April 28, 2024, 11:58 p.m. UTC | #4
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?
Sagi Grimberg April 30, 2024, noon UTC | #5
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.
Shinichiro Kawasaki May 3, 2024, 6:23 a.m. UTC | #6
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.
Sagi Grimberg May 3, 2024, 7:33 a.m. UTC | #7
>>> 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 mbox series

Patch

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() {