[dpdk-dev] [PATCH v1] Add check for skipped tests

[dpdk-dev] [PATCH v1] Add check for skipped tests

[Ayuj Verma]

Current unit test application add up skipped test (TEST_SKIPPED) into
FAILED test counter which is wrong. A testcase can be skipped for a
particular PMD if it doesn’t support specific feature whereas failure
mean PMD support feature but test run failed. Since TEST_SKIPPED
and TEST_FAILED serve different purpose, thus add a new counter
skipped in test app to count number of skipped test.

Ayuj Verma (1):
  app/test: add check for test skipped

 app/test/test.c | 2 ++
 1 file changed, 2 insertions(+)

-- 
1.8.3.1

[dpdk-dev] [PATCH v1] Add check for skipped tests

[Ayuj Verma]

Current unit test application add up skipped test (TEST_SKIPPED) into
FAILED test counter which is wrong. A testcase can be skipped for a
particular PMD if it doesn’t support specific feature whereas failure
mean PMD support feature but test run failed. Since TEST_SKIPPED
and TEST_FAILED serve different purpose, thus add a new counter
skipped in test app to count number of skipped test.

Ayuj Verma (1):
  app/test: add check for test skipped

 app/test/test.c | 2 ++
 1 file changed, 2 insertions(+)

-- 
1.8.3.1

[dpdk-dev] [PATCH v1] app/test: add check for tests skipped

[Ayuj Verma]

Add skipped counter to count for number of skipped testcases.

Signed-off-by: Ayuj Verma <ayverma@marvell.com>
Signed-off-by: Shally Verma <shallyv@marvell.com>
---
 app/test/test.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/test/test.c b/app/test/test.c
index d646f51..1e0113b 100644
--- a/app/test/test.c
+++ b/app/test/test.c
@@ -237,6 +237,8 @@
 				succeeded++;
 			else if (test_success == -ENOTSUP)
 				unsupported++;
+			else if (test_success == TEST_SKIPPED)
+				skipped++;
 			else
 				failed++;
 		} else if (test_success == -ENOTSUP) {
-- 
1.8.3.1

[dpdk-dev] [PATCH v1] app/test: add check for tests skipped

[Ayuj Verma]

Add skipped counter to count for number of skipped testcases.

Signed-off-by: Ayuj Verma <ayverma@marvell.com>
Signed-off-by: Shally Verma <shallyv@marvell.com>
---
 app/test/test.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/test/test.c b/app/test/test.c
index d646f51..1e0113b 100644
--- a/app/test/test.c
+++ b/app/test/test.c
@@ -237,6 +237,8 @@
 				succeeded++;
 			else if (test_success == -ENOTSUP)
 				unsupported++;
+			else if (test_success == TEST_SKIPPED)
+				skipped++;
 			else
 				failed++;
 		} else if (test_success == -ENOTSUP) {
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v1] app/test: add check for tests skipped

[Ayuj Verma]

Hi Akhil,


Did you get chance to look into this.


Thanks and regards

Ayuj Verma

________________________________
From: Ayuj Verma <ayverma@marvell.com>
Sent: 01 April 2019 13:01:56
To: akhil.goyal@nxp.com
Cc: arkadiuszx.kusztal@intel.com; Shally Verma; Sunila Sahu; Kanaka Durga Kotamarthy; Arvind Desai; dev@dpdk.org; Ayuj Verma
Subject: [PATCH v1] app/test: add check for tests skipped

Add skipped counter to count for number of skipped testcases.

Signed-off-by: Ayuj Verma <ayverma@marvell.com>
Signed-off-by: Shally Verma <shallyv@marvell.com>
---
 app/test/test.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/test/test.c b/app/test/test.c
index d646f51..1e0113b 100644
--- a/app/test/test.c
+++ b/app/test/test.c
@@ -237,6 +237,8 @@
                                 succeeded++;
                         else if (test_success == -ENOTSUP)
                                 unsupported++;
+                       else if (test_success == TEST_SKIPPED)
+                               skipped++;
                         else
                                 failed++;
                 } else if (test_success == -ENOTSUP) {
--
1.8.3.1

Re: [dpdk-dev] [PATCH v1] app/test: add check for tests skipped

[Ayuj Verma]

Hi Akhil,


Did you get chance to look into this.


Thanks and regards

Ayuj Verma

________________________________
From: Ayuj Verma <ayverma@marvell.com>
Sent: 01 April 2019 13:01:56
To: akhil.goyal@nxp.com
Cc: arkadiuszx.kusztal@intel.com; Shally Verma; Sunila Sahu; Kanaka Durga Kotamarthy; Arvind Desai; dev@dpdk.org; Ayuj Verma
Subject: [PATCH v1] app/test: add check for tests skipped

Add skipped counter to count for number of skipped testcases.

Signed-off-by: Ayuj Verma <ayverma@marvell.com>
Signed-off-by: Shally Verma <shallyv@marvell.com>
---
 app/test/test.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/test/test.c b/app/test/test.c
index d646f51..1e0113b 100644
--- a/app/test/test.c
+++ b/app/test/test.c
@@ -237,6 +237,8 @@
                                 succeeded++;
                         else if (test_success == -ENOTSUP)
                                 unsupported++;
+                       else if (test_success == TEST_SKIPPED)
+                               skipped++;
                         else
                                 failed++;
                 } else if (test_success == -ENOTSUP) {
--
1.8.3.1

Re: [dpdk-dev] [PATCH v1] app/test: add check for tests skipped

[Ayuj Verma]

+ Fiona and Pablo


Hi Akhil, Arek, Fiona and Pablo


Did you get a chance to look into this.


Thanks and regards

Ayuj Verma


________________________________
From: Ayuj Verma <ayverma@marvell.com>
Sent: 01 April 2019 13:01:56
To: akhil.goyal@nxp.com
Cc: arkadiuszx.kusztal@intel.com; Shally Verma; Sunila Sahu; Kanaka Durga Kotamarthy; Arvind Desai; dev@dpdk.org; Ayuj Verma
Subject: [PATCH v1] app/test: add check for tests skipped

Add skipped counter to count for number of skipped testcases.

Signed-off-by: Ayuj Verma <ayverma@marvell.com>
Signed-off-by: Shally Verma <shallyv@marvell.com>
---
 app/test/test.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/test/test.c b/app/test/test.c
index d646f51..1e0113b 100644
--- a/app/test/test.c
+++ b/app/test/test.c
@@ -237,6 +237,8 @@
                                 succeeded++;
                         else if (test_success == -ENOTSUP)
                                 unsupported++;
+                       else if (test_success == TEST_SKIPPED)
+                               skipped++;
                         else
                                 failed++;
                 } else if (test_success == -ENOTSUP) {
--
1.8.3.1

Re: [dpdk-dev] [PATCH v1] app/test: add check for tests skipped

[Ayuj Verma]

+ Fiona and Pablo


Hi Akhil, Arek, Fiona and Pablo


Did you get a chance to look into this.


Thanks and regards

Ayuj Verma


________________________________
From: Ayuj Verma <ayverma@marvell.com>
Sent: 01 April 2019 13:01:56
To: akhil.goyal@nxp.com
Cc: arkadiuszx.kusztal@intel.com; Shally Verma; Sunila Sahu; Kanaka Durga Kotamarthy; Arvind Desai; dev@dpdk.org; Ayuj Verma
Subject: [PATCH v1] app/test: add check for tests skipped

Add skipped counter to count for number of skipped testcases.

Signed-off-by: Ayuj Verma <ayverma@marvell.com>
Signed-off-by: Shally Verma <shallyv@marvell.com>
---
 app/test/test.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/test/test.c b/app/test/test.c
index d646f51..1e0113b 100644
--- a/app/test/test.c
+++ b/app/test/test.c
@@ -237,6 +237,8 @@
                                 succeeded++;
                         else if (test_success == -ENOTSUP)
                                 unsupported++;
+                       else if (test_success == TEST_SKIPPED)
+                               skipped++;
                         else
                                 failed++;
                 } else if (test_success == -ENOTSUP) {
--
1.8.3.1

Re: [dpdk-dev] [PATCH v1] app/test: add check for tests skipped

[Trahe, Fiona]

Hi Ayuj,

________________________________________
From: Ayuj Verma <ayverma@marvell.com>
Sent: 01 April 2019 13:01:56
To: akhil.goyal@nxp.com
Cc: arkadiuszx.kusztal@intel.com; Shally Verma; Sunila Sahu; Kanaka Durga Kotamarthy; Arvind Desai; dev@dpdk.org; Ayuj Verma
Subject: [PATCH v1] app/test: add check for tests skipped 
 
Add skipped counter to count for number of skipped testcases.

Signed-off-by: Ayuj Verma <ayverma@marvell.com>
Signed-off-by: Shally Verma <shallyv@marvell.com>
---
 app/test/test.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/test/test.c b/app/test/test.c
index d646f51..1e0113b 100644
--- a/app/test/test.c
+++ b/app/test/test.c
@@ -237,6 +237,8 @@
                                 succeeded++;
                         else if (test_success == -ENOTSUP)
                                 unsupported++;
+                       else if (test_success == TEST_SKIPPED)
+                               skipped++;
                         else
                                 failed++;
                 } else if (test_success == -ENOTSUP) {
-- 
1.8.3.1

[Fiona] I just tried it out.
The patch itself is ok - it works if a test case at run-time decides to return TEST_SKIPPED.
A compile-time decision to skip a test-case can be made by using TEST_CASE_ST_DISABLED and this already counts as skipped.

But I see the 2 places you're using TEST_SKIPPED are where the device doesn't support the capability needed for the test. In other similar cases (e.g. sym_crypto ZUC test) -ENOTSUP is returned. This seems like a more appropriate return value.
So unless there's some other reason other than Unsupported that a run-time decision would be made to skip a test, I'd nack this patch as it leads to confusion about which value should be returned.

Fiona

Re: [dpdk-dev] [PATCH v1] app/test: add check for tests skipped

[Trahe, Fiona]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="UTF-8", Size: 1829 bytes --]

Hi Ayuj,

________________________________________
From: Ayuj Verma <ayverma@marvell.com>
Sent: 01 April 2019 13:01:56
To: akhil.goyal@nxp.com
Cc: arkadiuszx.kusztal@intel.com; Shally Verma; Sunila Sahu; Kanaka Durga Kotamarthy; Arvind Desai; dev@dpdk.org; Ayuj Verma
Subject: [PATCH v1] app/test: add check for tests skipped 
 
Add skipped counter to count for number of skipped testcases.

Signed-off-by: Ayuj Verma <ayverma@marvell.com>
Signed-off-by: Shally Verma <shallyv@marvell.com>
---
 app/test/test.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/test/test.c b/app/test/test.c
index d646f51..1e0113b 100644
--- a/app/test/test.c
+++ b/app/test/test.c
@@ -237,6 +237,8 @@
                                 succeeded++;
                         else if (test_success == -ENOTSUP)
                                 unsupported++;
+                       else if (test_success == TEST_SKIPPED)
+                               skipped++;
                         else
                                 failed++;
                 } else if (test_success == -ENOTSUP) {
-- 
1.8.3.1

[Fiona] I just tried it out.
The patch itself is ok - it works if a test case at run-time decides to return TEST_SKIPPED.
A compile-time decision to skip a test-case can be made by using TEST_CASE_ST_DISABLED and this already counts as skipped.

But I see the 2 places you're using TEST_SKIPPED are where the device doesn't support the capability needed for the test. In other similar cases (e.g. sym_crypto ZUC test) -ENOTSUP is returned. This seems like a more appropriate return value.
So unless there's some other reason other than Unsupported that a run-time decision would be made to skip a test, I'd nack this patch as it leads to confusion about which value should be returned.

Fiona