[dpdk-ci] [PATCH v2 1/4] create_new_execution_file_from_tags: change tag argument

[dpdk-ci] [PATCH v2 1/4] create_new_execution_file_from_tags: change tag argument

[ohilyard]

From: Owen Hilyard <ohilyard@iol.unh.edu>

BREAKING CHANGE: --tags syntax changed

Due to limitations of the argparse library, you can't have 2 varidic
arguments. Since upcoming requirements for the CI require the ability to
specify individual tests to be run (ex: weekly rte_flow testing), this
script must have the ability to pass in those tests as well. This is a
change made to support that.

Old syntax:
"--tags core documentation"

New syntax:
"--tag core --tag documentation"

Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
---
 tools/create_new_execution_file_from_tags.py | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/tools/create_new_execution_file_from_tags.py b/tools/create_new_execution_file_from_tags.py
index d1d4447..b88fa93 100755
--- a/tools/create_new_execution_file_from_tags.py
+++ b/tools/create_new_execution_file_from_tags.py
@@ -73,7 +73,8 @@ def __str__(self):
     parser.add_argument('output_path', help='The path to the output execution file')
     parser.add_argument('testing_type', type=TestingType, choices=list(TestingType),
                         help='What type of testing to create an execution file for')
-    parser.add_argument('tags', metavar='tag', type=str, nargs='*', help='The tags to create an execution file for.')
+    parser.add_argument('--tag', type=str, action='append',
+                        help='The tags to create an execution file for.')
 
     args = parser.parse_args()
 
@@ -86,7 +87,9 @@ def __str__(self):
     test_map = {key: parse_comma_delimited_list_from_string(value.strip()) for key, value in
                 tag_to_test_map_parser[str(args.testing_type)].items()}
 
-    tests = map_tags_to_tests(args.tags, test_map)
+    tests = set()
+    if args.tag is not None:
+        tests = map_tags_to_tests(args.tag, test_map)
 
     try:
         output_file = open(args.output_path, 'x')
@@ -98,7 +101,11 @@ def __str__(self):
         if execution_plan != 'DEFAULT':
             test_allowlist = parse_comma_delimited_list_from_string(
                 template_execution_file_parser[execution_plan]['test_suites'])
-            tests_to_run = list(set(test_allowlist).intersection(tests))
+            if len(tests) != 0:
+                tests_to_run = list(set(test_allowlist).intersection(tests))
+            else:  # no tags given
+                tests_to_run = [entry for entry in test_allowlist if entry != '']
+
             tests_to_run.sort()
             template_execution_file_parser[execution_plan]['test_suites'] = ", ".join(tests_to_run)
 
-- 
2.30.2

[dpdk-ci] [PATCH v2 2/4] create_new_execution_file_from_tags: add test argument

[ohilyard]

From: Owen Hilyard <ohilyard@iol.unh.edu>

Adds an argument to allow passing in individual test cases to be added
to the resulting DTS execution file. This change was made to facilitate
periodic testing of testcases not currently in CI, such as rte_flow. A
test passed in using this argument will bypass the normal behavior of
only allowing test cases specified in the template execution file. This
behavior because the mapping of DPDK files to tags is not complete, and
to avoid not testing patches, the decision was made to run all tests if
the tests for a patch could not be determined.

Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
---
 tools/create_new_execution_file_from_tags.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/create_new_execution_file_from_tags.py b/tools/create_new_execution_file_from_tags.py
index b88fa93..61d801d 100755
--- a/tools/create_new_execution_file_from_tags.py
+++ b/tools/create_new_execution_file_from_tags.py
@@ -75,6 +75,8 @@ def __str__(self):
                         help='What type of testing to create an execution file for')
     parser.add_argument('--tag', type=str, action='append',
                         help='The tags to create an execution file for.')
+    parser.add_argument('--test', type=str, action='append',
+                        help='The tests to run along with the tests required by the provided tags')
 
     args = parser.parse_args()
 
@@ -106,6 +108,10 @@ def __str__(self):
             else:  # no tags given
                 tests_to_run = [entry for entry in test_allowlist if entry != '']
 
+            if args.test is not None:
+                for test in args.test:
+                    tests_to_run.append(test)
+
             tests_to_run.sort()
             template_execution_file_parser[execution_plan]['test_suites'] = ", ".join(tests_to_run)
 
-- 
2.30.2

[dpdk-ci] [PATCH v2 3/4] guess_git_tree: module doc comment moved

[ohilyard]

From: Owen Hilyard <ohilyard@iol.unh.edu>

The import statements in the file have been moved below the module
doc comment per PEP 257.

Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
---
 tools/guess_git_tree.py | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/tools/guess_git_tree.py b/tools/guess_git_tree.py
index c9eef39..e186f5e 100755
--- a/tools/guess_git_tree.py
+++ b/tools/guess_git_tree.py
@@ -1,20 +1,8 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 
 # SPDX-License-Identifier: (BSD-3-Clause AND GPL-2.0-or-later AND MIT)
 # Copyright 2019 Mellanox Technologies, Ltd
 
-import os
-import sys
-import re
-import argparse
-import fnmatch
-
-from requests.exceptions import HTTPError
-
-from git_pw import config
-from git_pw import api
-from git_pw import utils
-
 """
 Description:
 This script uses the git-pw API to retrieve Patchwork's
@@ -49,6 +37,17 @@
     tree = maintainers.get_tree(files)
 """
 
+import os
+import sys
+import re
+import argparse
+import fnmatch
+
+from requests.exceptions import HTTPError
+
+from git_pw import config
+from git_pw import api
+from git_pw import utils
 
 MAINTAINERS_FILE_PATH = os.environ.get('MAINTAINERS_FILE_PATH')
 if not MAINTAINERS_FILE_PATH:
-- 
2.30.2

[dpdk-ci] [PATCH v2 4/4] guess_git_tree: fix crash caused by empty diff

[ohilyard]

From: Owen Hilyard <ohilyard@iol.unh.edu>

A sanity check has been added to find_filenames. Occasionally, due to
how the community lab internally handles getting patches from
patchworks, a patch will result in no diff. This patch adds handling for
this case.

Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
---
 tools/guess_git_tree.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/guess_git_tree.py b/tools/guess_git_tree.py
index e186f5e..63892e1 100755
--- a/tools/guess_git_tree.py
+++ b/tools/guess_git_tree.py
@@ -92,6 +92,11 @@ def find_filenames(diff):
             - Moved _filename_re into the method.
             - Reduced newlines.
         """
+        # sanity check diff
+        # for patches without any diff, it will try to run diff.replace
+        # while diff is None. just return an empty list
+        if diff is None:
+            return []
         _filename_re = re.compile(r'^(---|\+\+\+) (\S+)')
         # normalise spaces
         diff = diff.replace('\r', '')
-- 
2.30.2

RE: [PATCH v2 1/4] create_new_execution_file_from_tags: change tag argument

[Ali Alnubani]

Hi Owen,

> -----Original Message-----
> From: ohilyard@iol.unh.edu <ohilyard@iol.unh.edu>
> Sent: Wednesday, October 13, 2021 5:00 PM
> To: ci@dpdk.org
> Cc: aconole@redhat.com; Ali Alnubani <alialnu@nvidia.com>; Owen Hilyard
> <ohilyard@iol.unh.edu>
> Subject: [PATCH v2 1/4] create_new_execution_file_from_tags: change tag
> argument
> 

Thank you for addressing my comment in v1. Can you please rebase this patchset on latest?

Thanks,
Ali

[PATCH v3 1/4] create_new_execution_file_from_tags: change tag argument

[ohilyard]

From: Owen Hilyard <ohilyard@iol.unh.edu>

BREAKING CHANGE: --tags syntax changed

Due to limitations of the argparse library, you can't have 2 varidic
arguments. Since upcoming requirements for the CI require the ability to
specify individual tests to be run (ex: weekly rte_flow testing), this
script must have the ability to pass in those tests as well. This is a
change made to support that.

Old syntax:
"--tags core documentation"

New syntax:
"--tag core --tag documentation"

Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
---
 tools/create_new_execution_file_from_tags.py | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/tools/create_new_execution_file_from_tags.py b/tools/create_new_execution_file_from_tags.py
index d1d4447..60ad663 100755
--- a/tools/create_new_execution_file_from_tags.py
+++ b/tools/create_new_execution_file_from_tags.py
@@ -73,7 +73,8 @@ if __name__ == '__main__':
     parser.add_argument('output_path', help='The path to the output execution file')
     parser.add_argument('testing_type', type=TestingType, choices=list(TestingType),
                         help='What type of testing to create an execution file for')
-    parser.add_argument('tags', metavar='tag', type=str, nargs='*', help='The tags to create an execution file for.')
+    parser.add_argument('--tag', type=str, action='append',
+                        help='The tags to create an execution file for.')
 
     args = parser.parse_args()
 
@@ -86,7 +87,9 @@ if __name__ == '__main__':
     test_map = {key: parse_comma_delimited_list_from_string(value.strip()) for key, value in
                 tag_to_test_map_parser[str(args.testing_type)].items()}
 
-    tests = map_tags_to_tests(args.tags, test_map)
+    tests = set()
+    if args.tag is not None:
+        tests = map_tags_to_tests(args.tags, test_map)
 
     try:
         output_file = open(args.output_path, 'x')
@@ -98,7 +101,11 @@ if __name__ == '__main__':
         if execution_plan != 'DEFAULT':
             test_allowlist = parse_comma_delimited_list_from_string(
                 template_execution_file_parser[execution_plan]['test_suites'])
-            tests_to_run = list(set(test_allowlist).intersection(tests))
+            if len(tests) != 0:
+                tests_to_run = list(set(test_allowlist).intersection(tests))
+            else:
+                tests_to_run = [entry for entry in test_allowlist if entry != '']
+                
             tests_to_run.sort()
             template_execution_file_parser[execution_plan]['test_suites'] = ", ".join(tests_to_run)
 
-- 
2.30.2

RE: [PATCH v3 1/4] create_new_execution_file_from_tags: change tag argument

[Ali Alnubani]

> -----Original Message-----
> From: ohilyard@iol.unh.edu <ohilyard@iol.unh.edu>
> Sent: Tuesday, February 1, 2022 11:36 PM
> To: ci@dpdk.org; Ali Alnubani <alialnu@nvidia.com>
> Cc: Owen Hilyard <ohilyard@iol.unh.edu>
> Subject: [PATCH v3 1/4] create_new_execution_file_from_tags: change tag
> argument
> 
[..]
> @@ -98,7 +101,11 @@ if __name__ == '__main__':
>          if execution_plan != 'DEFAULT':
>              test_allowlist = parse_comma_delimited_list_from_string(
>                  template_execution_file_parser[execution_plan]['test_suites'])
> -            tests_to_run = list(set(test_allowlist).intersection(tests))
> +            if len(tests) != 0:
> +                tests_to_run = list(set(test_allowlist).intersection(tests))
> +            else:
> +                tests_to_run = [entry for entry in test_allowlist if entry != '']
> +

There are empty spaces in this line.

[PATCH v4 1/4] create_new_execution_file_from_tags: change tag argument

[ohilyard]

From: Owen Hilyard <ohilyard@iol.unh.edu>

BREAKING CHANGE: --tags syntax changed

Due to limitations of the argparse library, you can't have 2 varidic
arguments. Since upcoming requirements for the CI require the ability to
specify individual tests to be run (ex: weekly rte_flow testing), this
script must have the ability to pass in those tests as well. This is a
change made to support that.

Old syntax:
"--tags core documentation"

New syntax:
"--tag core --tag documentation"

Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
---
 tools/create_new_execution_file_from_tags.py | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/tools/create_new_execution_file_from_tags.py b/tools/create_new_execution_file_from_tags.py
index d1d4447..60ad663 100755
--- a/tools/create_new_execution_file_from_tags.py
+++ b/tools/create_new_execution_file_from_tags.py
@@ -73,7 +73,8 @@ if __name__ == '__main__':
     parser.add_argument('output_path', help='The path to the output execution file')
     parser.add_argument('testing_type', type=TestingType, choices=list(TestingType),
                         help='What type of testing to create an execution file for')
-    parser.add_argument('tags', metavar='tag', type=str, nargs='*', help='The tags to create an execution file for.')
+    parser.add_argument('--tag', type=str, action='append',
+                        help='The tags to create an execution file for.')
 
     args = parser.parse_args()
 
@@ -86,7 +87,9 @@ if __name__ == '__main__':
     test_map = {key: parse_comma_delimited_list_from_string(value.strip()) for key, value in
                 tag_to_test_map_parser[str(args.testing_type)].items()}
 
-    tests = map_tags_to_tests(args.tags, test_map)
+    tests = set()
+    if args.tag is not None:
+        tests = map_tags_to_tests(args.tags, test_map)
 
     try:
         output_file = open(args.output_path, 'x')
@@ -98,7 +101,11 @@ if __name__ == '__main__':
         if execution_plan != 'DEFAULT':
             test_allowlist = parse_comma_delimited_list_from_string(
                 template_execution_file_parser[execution_plan]['test_suites'])
-            tests_to_run = list(set(test_allowlist).intersection(tests))
+            if len(tests) != 0:
+                tests_to_run = list(set(test_allowlist).intersection(tests))
+            else:
+                tests_to_run = [entry for entry in test_allowlist if entry != '']
+                
             tests_to_run.sort()
             template_execution_file_parser[execution_plan]['test_suites'] = ", ".join(tests_to_run)
 
-- 
2.30.2

[PATCH v4 2/4] create_new_execution_file_from_tags: add test argument

[ohilyard]

From: Owen Hilyard <ohilyard@iol.unh.edu>

Adds an argument to allow passing in individual test cases to be added
to the resulting DTS execution file. This change was made to facilitate
periodic testing of testcases not currently in CI, such as rte_flow. A
test passed in using this argument will bypass the normal behavior of
only allowing test cases specified in the template execution file. This
behavior because the mapping of DPDK files to tags is not complete, and
to avoid not testing patches, the decision was made to run all tests if
the tests for a patch could not be determined.

Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
---
 tools/create_new_execution_file_from_tags.py | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/create_new_execution_file_from_tags.py b/tools/create_new_execution_file_from_tags.py
index 60ad663..58bcf1a 100755
--- a/tools/create_new_execution_file_from_tags.py
+++ b/tools/create_new_execution_file_from_tags.py
@@ -75,6 +75,8 @@ if __name__ == '__main__':
                         help='What type of testing to create an execution file for')
     parser.add_argument('--tag', type=str, action='append',
                         help='The tags to create an execution file for.')
+    parser.add_argument('--test', type=str, action='append',
+                        help='The tests to run along with the tests required by the provided tags')
 
     args = parser.parse_args()
 
@@ -105,7 +107,11 @@ if __name__ == '__main__':
                 tests_to_run = list(set(test_allowlist).intersection(tests))
             else:
                 tests_to_run = [entry for entry in test_allowlist if entry != '']
-                
+
+            if args.test is not None:
+                for test in args.test:
+                    tests_to_run.append(test)
+
             tests_to_run.sort()
             template_execution_file_parser[execution_plan]['test_suites'] = ", ".join(tests_to_run)
 
-- 
2.30.2

[PATCH v4 3/4] pw_maintainers_cli: move module doc comment

[ohilyard]

From: Owen Hilyard <ohilyard@iol.unh.edu>

The import statements in the file have been moved below the module
doc comment per PEP 257.

Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
---
 tools/pw_maintainers_cli.py | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/tools/pw_maintainers_cli.py b/tools/pw_maintainers_cli.py
index fd69081..0705eee 100755
--- a/tools/pw_maintainers_cli.py
+++ b/tools/pw_maintainers_cli.py
@@ -3,19 +3,6 @@
 # SPDX-License-Identifier: (BSD-3-Clause AND GPL-2.0-or-later AND MIT)
 # Copyright 2019 Mellanox Technologies, Ltd
 
-import os
-import sys
-import re
-import argparse
-import fnmatch
-
-from requests.exceptions import HTTPError
-
-from git_pw import config
-from git_pw import api
-from git_pw import utils
-from git_pw import patch as git_pw_patch
-
 """
 Description:
 This script uses the git-pw API to retrieve Patchwork's
@@ -53,6 +40,18 @@ Or if you want to use inside other scripts:
     maintainers = maintainers.get_maintainers(tree_url)
 """
 
+import os
+import sys
+import re
+import argparse
+import fnmatch
+
+from requests.exceptions import HTTPError
+
+from git_pw import config
+from git_pw import api
+from git_pw import utils
+from git_pw import patch as git_pw_patch
 
 MAINTAINERS_FILE_PATH = os.environ.get('MAINTAINERS_FILE_PATH')
 if not MAINTAINERS_FILE_PATH:
-- 
2.30.2

[PATCH v4 4/4] pw_maintainers_cli: fix crash caused by empty diff

[ohilyard]

From: Owen Hilyard <ohilyard@iol.unh.edu>

A sanity check has been added to find_filenames. Occasionally, due to
how the community lab internally handles getting patches from
patchworks, a patch will result in no diff. This patch adds handling for
this case.

Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
---
 tools/pw_maintainers_cli.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/pw_maintainers_cli.py b/tools/pw_maintainers_cli.py
index 0705eee..975b62b 100755
--- a/tools/pw_maintainers_cli.py
+++ b/tools/pw_maintainers_cli.py
@@ -124,6 +124,11 @@ class Diff(object):
             - Moved _filename_re into the method.
             - Reduced newlines.
         """
+        # sanity check diff
+        # for patches without any diff, it will try to run diff.replace
+        # while diff is None. just return an empty list
+        if diff is None:
+            return []
         _filename_re = re.compile(r'^(---|\+\+\+) (\S+)')
         # normalise spaces
         diff = diff.replace('\r', '')
-- 
2.30.2

Re: [PATCH v4 1/4] create_new_execution_file_from_tags: change tag argument

[Owen Hilyard]

[-- Attachment #1: Type: text/plain, Size: 33 bytes --]

V4 should address your feedback.

[-- Attachment #2: Type: text/html, Size: 78 bytes --]

[PATCH v5 1/3] create_new_execution_file_from_tags: change tag argument

[ohilyard]

From: Owen Hilyard <ohilyard@iol.unh.edu>

BREAKING CHANGE: --tags syntax changed

Due to limitations of the argparse library, you can't have 2 varidic
arguments. Since upcoming requirements for the CI require the ability to
specify individual tests to be run (ex: weekly rte_flow testing), this
script must have the ability to pass in those tests as well. This is a
change made to support that.

Old syntax:
"--tags core documentation"

New syntax:
"--tag core --tag documentation"

Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
---
 tools/create_new_execution_file_from_tags.py | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/tools/create_new_execution_file_from_tags.py b/tools/create_new_execution_file_from_tags.py
index d1d4447..f2081ee 100755
--- a/tools/create_new_execution_file_from_tags.py
+++ b/tools/create_new_execution_file_from_tags.py
@@ -73,7 +73,10 @@ if __name__ == '__main__':
     parser.add_argument('output_path', help='The path to the output execution file')
     parser.add_argument('testing_type', type=TestingType, choices=list(TestingType),
                         help='What type of testing to create an execution file for')
-    parser.add_argument('tags', metavar='tag', type=str, nargs='*', help='The tags to create an execution file for.')
+    parser.add_argument('--tag', type=str, action='append',
+                        help='The tags to create an execution file for.')
+    parser.add_argument('--test', type=str, action='append',
+                        help='The tests to run along with the tests required by the provided tags')
 
     args = parser.parse_args()
 
@@ -86,7 +89,9 @@ if __name__ == '__main__':
     test_map = {key: parse_comma_delimited_list_from_string(value.strip()) for key, value in
                 tag_to_test_map_parser[str(args.testing_type)].items()}
 
-    tests = map_tags_to_tests(args.tags, test_map)
+    tests = set()
+    if args.tag is not None:
+        tests = map_tags_to_tests(args.tags, test_map)
 
     try:
         output_file = open(args.output_path, 'x')
@@ -98,7 +103,14 @@ if __name__ == '__main__':
         if execution_plan != 'DEFAULT':
             test_allowlist = parse_comma_delimited_list_from_string(
                 template_execution_file_parser[execution_plan]['test_suites'])
-            tests_to_run = list(set(test_allowlist).intersection(tests))
+            if len(tests) != 0:
+                tests_to_run = list(set(test_allowlist).intersection(tests))
+            else:
+                tests_to_run = [entry for entry in test_allowlist if entry != '']
+
+            if args.test is not None:
+                for test in args.test:
+                    tests_to_run.append(test)
             tests_to_run.sort()
             template_execution_file_parser[execution_plan]['test_suites'] = ", ".join(tests_to_run)
 
-- 
2.30.2

[PATCH v5 2/3] pw_maintainers_cli: move module doc comment

[ohilyard]

From: Owen Hilyard <ohilyard@iol.unh.edu>

The import statements in the file have been moved below the module
doc comment per PEP 257.

Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
---
 tools/pw_maintainers_cli.py | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/tools/pw_maintainers_cli.py b/tools/pw_maintainers_cli.py
index fd69081..0705eee 100755
--- a/tools/pw_maintainers_cli.py
+++ b/tools/pw_maintainers_cli.py
@@ -3,19 +3,6 @@
 # SPDX-License-Identifier: (BSD-3-Clause AND GPL-2.0-or-later AND MIT)
 # Copyright 2019 Mellanox Technologies, Ltd
 
-import os
-import sys
-import re
-import argparse
-import fnmatch
-
-from requests.exceptions import HTTPError
-
-from git_pw import config
-from git_pw import api
-from git_pw import utils
-from git_pw import patch as git_pw_patch
-
 """
 Description:
 This script uses the git-pw API to retrieve Patchwork's
@@ -53,6 +40,18 @@ Or if you want to use inside other scripts:
     maintainers = maintainers.get_maintainers(tree_url)
 """
 
+import os
+import sys
+import re
+import argparse
+import fnmatch
+
+from requests.exceptions import HTTPError
+
+from git_pw import config
+from git_pw import api
+from git_pw import utils
+from git_pw import patch as git_pw_patch
 
 MAINTAINERS_FILE_PATH = os.environ.get('MAINTAINERS_FILE_PATH')
 if not MAINTAINERS_FILE_PATH:
-- 
2.30.2

RE: [PATCH v5 2/3] pw_maintainers_cli: move module doc comment

[Ali Alnubani]

> -----Original Message-----
> From: ohilyard@iol.unh.edu <ohilyard@iol.unh.edu>
> Sent: Monday, February 7, 2022 5:08 PM
> To: ci@dpdk.org; Ali Alnubani <alialnu@nvidia.com>
> Cc: Owen Hilyard <ohilyard@iol.unh.edu>
> Subject: [PATCH v5 2/3] pw_maintainers_cli: move module doc comment
> 
> From: Owen Hilyard <ohilyard@iol.unh.edu>
> 
> The import statements in the file have been moved below the module
> doc comment per PEP 257.
> 
> Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> ---

Thanks.

Tested-by: Ali Alnubani <alialnu@nvidia.com>
Acked-by: Ali Alnubani <alialnu@nvidia.com>

[PATCH v5 3/3] pw_maintainers_cli: fix crash caused by empty diff

[ohilyard]

From: Owen Hilyard <ohilyard@iol.unh.edu>

A sanity check has been added to find_filenames. Occasionally, due to
how the community lab internally handles getting patches from
patchworks, a patch will result in no diff. This patch adds handling for
this case.

Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
---
 tools/pw_maintainers_cli.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/pw_maintainers_cli.py b/tools/pw_maintainers_cli.py
index 0705eee..975b62b 100755
--- a/tools/pw_maintainers_cli.py
+++ b/tools/pw_maintainers_cli.py
@@ -124,6 +124,11 @@ class Diff(object):
             - Moved _filename_re into the method.
             - Reduced newlines.
         """
+        # sanity check diff
+        # for patches without any diff, it will try to run diff.replace
+        # while diff is None. just return an empty list
+        if diff is None:
+            return []
         _filename_re = re.compile(r'^(---|\+\+\+) (\S+)')
         # normalise spaces
         diff = diff.replace('\r', '')
-- 
2.30.2

RE: [PATCH v5 3/3] pw_maintainers_cli: fix crash caused by empty diff

[Ali Alnubani]

> -----Original Message-----
> From: ohilyard@iol.unh.edu <ohilyard@iol.unh.edu>
> Sent: Monday, February 7, 2022 5:08 PM
> To: ci@dpdk.org; Ali Alnubani <alialnu@nvidia.com>
> Cc: Owen Hilyard <ohilyard@iol.unh.edu>
> Subject: [PATCH v5 3/3] pw_maintainers_cli: fix crash caused by empty diff
> 
> From: Owen Hilyard <ohilyard@iol.unh.edu>
> 
> A sanity check has been added to find_filenames. Occasionally, due to
> how the community lab internally handles getting patches from
> patchworks, a patch will result in no diff. This patch adds handling for
> this case.
> 
> Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> ---

Thanks Owen.

Tested-by: Ali Alnubani <alialnu@nvidia.com>
Acked-by: Ali Alnubani <alialnu@nvidia.com>

RE: [PATCH v5 3/3] pw_maintainers_cli: fix crash caused by empty diff

[Ali Alnubani]

> -----Original Message-----
> From: Ali Alnubani <alialnu@nvidia.com>
> Sent: Monday, February 7, 2022 5:56 PM
> To: ohilyard@iol.unh.edu; ci@dpdk.org
> Subject: RE: [PATCH v5 3/3] pw_maintainers_cli: fix crash caused by empty diff
> 
> > -----Original Message-----
> > From: ohilyard@iol.unh.edu <ohilyard@iol.unh.edu>
> > Sent: Monday, February 7, 2022 5:08 PM
> > To: ci@dpdk.org; Ali Alnubani <alialnu@nvidia.com>
> > Cc: Owen Hilyard <ohilyard@iol.unh.edu>
> > Subject: [PATCH v5 3/3] pw_maintainers_cli: fix crash caused by empty diff
> >
> > From: Owen Hilyard <ohilyard@iol.unh.edu>
> >
> > A sanity check has been added to find_filenames. Occasionally, due to
> > how the community lab internally handles getting patches from
> > patchworks, a patch will result in no diff. This patch adds handling for
> > this case.
> >
> > Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> > ---
> 
> Thanks Owen.
> 
> Tested-by: Ali Alnubani <alialnu@nvidia.com>
> Acked-by: Ali Alnubani <alialnu@nvidia.com>

Hi Thomas and Aaron,

Can we merge this patch? Pull requests can still cause the script to crash because they have an empty diff.

Patrick, are [1/3] and [2/3] still needed as well?

Thanks,
Ali

Re: [PATCH v5 3/3] pw_maintainers_cli: fix crash caused by empty diff

[Patrick Robb]

[-- Attachment #1: Type: text/plain, Size: 1778 bytes --]

>
> Patrick, are [1/3] and [2/3] still needed as well?
>
Yes please on both. We are currently using 1/3 in our Jenkins process for
including tag arguments in creating the execution file. Thanks for raising
this.



On Wed, Jun 21, 2023 at 11:19 AM Ali Alnubani <alialnu@nvidia.com> wrote:

> > -----Original Message-----
> > From: Ali Alnubani <alialnu@nvidia.com>
> > Sent: Monday, February 7, 2022 5:56 PM
> > To: ohilyard@iol.unh.edu; ci@dpdk.org
> > Subject: RE: [PATCH v5 3/3] pw_maintainers_cli: fix crash caused by
> empty diff
> >
> > > -----Original Message-----
> > > From: ohilyard@iol.unh.edu <ohilyard@iol.unh.edu>
> > > Sent: Monday, February 7, 2022 5:08 PM
> > > To: ci@dpdk.org; Ali Alnubani <alialnu@nvidia.com>
> > > Cc: Owen Hilyard <ohilyard@iol.unh.edu>
> > > Subject: [PATCH v5 3/3] pw_maintainers_cli: fix crash caused by empty
> diff
> > >
> > > From: Owen Hilyard <ohilyard@iol.unh.edu>
> > >
> > > A sanity check has been added to find_filenames. Occasionally, due to
> > > how the community lab internally handles getting patches from
> > > patchworks, a patch will result in no diff. This patch adds handling
> for
> > > this case.
> > >
> > > Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> > > ---
> >
> > Thanks Owen.
> >
> > Tested-by: Ali Alnubani <alialnu@nvidia.com>
> > Acked-by: Ali Alnubani <alialnu@nvidia.com>
>
> Hi Thomas and Aaron,
>
> Can we merge this patch? Pull requests can still cause the script to crash
> because they have an empty diff.
>
> Patrick, are [1/3] and [2/3] still needed as well?
>
> Thanks,
> Ali
>


-- 

Patrick Robb

Technical Service Manager

UNH InterOperability Laboratory

21 Madbury Rd, Suite 100, Durham, NH 03824

www.iol.unh.edu

[-- Attachment #2: Type: text/html, Size: 5027 bytes --]

Re: [PATCH v5 3/3] pw_maintainers_cli: fix crash caused by empty diff

[Aaron Conole]

Patrick Robb <probb@iol.unh.edu> writes:

>  Patrick, are [1/3] and [2/3] still needed as well?
>
> Yes please on both. We are currently using 1/3 in our Jenkins process for including tag arguments in creating
> the execution file. Thanks for raising this.

Looks like this slipped my queue.  I'll apply it now.

Thanks.

>  
>
> On Wed, Jun 21, 2023 at 11:19 AM Ali Alnubani <alialnu@nvidia.com> wrote:
>
>  > -----Original Message-----
>  > From: Ali Alnubani <alialnu@nvidia.com>
>  > Sent: Monday, February 7, 2022 5:56 PM
>  > To: ohilyard@iol.unh.edu; ci@dpdk.org
>  > Subject: RE: [PATCH v5 3/3] pw_maintainers_cli: fix crash caused by empty diff
>  > 
>  > > -----Original Message-----
>  > > From: ohilyard@iol.unh.edu <ohilyard@iol.unh.edu>
>  > > Sent: Monday, February 7, 2022 5:08 PM
>  > > To: ci@dpdk.org; Ali Alnubani <alialnu@nvidia.com>
>  > > Cc: Owen Hilyard <ohilyard@iol.unh.edu>
>  > > Subject: [PATCH v5 3/3] pw_maintainers_cli: fix crash caused by empty diff
>  > >
>  > > From: Owen Hilyard <ohilyard@iol.unh.edu>
>  > >
>  > > A sanity check has been added to find_filenames. Occasionally, due to
>  > > how the community lab internally handles getting patches from
>  > > patchworks, a patch will result in no diff. This patch adds handling for
>  > > this case.
>  > >
>  > > Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
>  > > ---
>  > 
>  > Thanks Owen.
>  > 
>  > Tested-by: Ali Alnubani <alialnu@nvidia.com>
>  > Acked-by: Ali Alnubani <alialnu@nvidia.com>
>
>  Hi Thomas and Aaron,
>
>  Can we merge this patch? Pull requests can still cause the script to crash because they have an empty
>  diff.
>
>  Patrick, are [1/3] and [2/3] still needed as well?
>
>  Thanks,
>  Ali

RE: [PATCH v5 1/3] create_new_execution_file_from_tags: change tag argument

[Ali Alnubani]

> -----Original Message-----
> From: ohilyard@iol.unh.edu <ohilyard@iol.unh.edu>
> Sent: Monday, February 7, 2022 5:08 PM
> To: ci@dpdk.org; Ali Alnubani <alialnu@nvidia.com>
> Cc: Owen Hilyard <ohilyard@iol.unh.edu>
> Subject: [PATCH v5 1/3] create_new_execution_file_from_tags: change tag
> argument
> 
> From: Owen Hilyard <ohilyard@iol.unh.edu>
> 
> BREAKING CHANGE: --tags syntax changed
> 
> Due to limitations of the argparse library, you can't have 2 varidic
> arguments. Since upcoming requirements for the CI require the ability to
> specify individual tests to be run (ex: weekly rte_flow testing), this
> script must have the ability to pass in those tests as well. This is a
> change made to support that.
> 
> Old syntax:
> "--tags core documentation"
> 
> New syntax:
> "--tag core --tag documentation"
> 
> Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> ---

Thanks.

Acked-by: Ali Alnubani <alialnu@nvidia.com>

Re: [PATCH v5 1/3] create_new_execution_file_from_tags: change tag argument

[Aaron Conole]

ohilyard@iol.unh.edu writes:

> From: Owen Hilyard <ohilyard@iol.unh.edu>
>
> BREAKING CHANGE: --tags syntax changed
>
> Due to limitations of the argparse library, you can't have 2 varidic
> arguments. Since upcoming requirements for the CI require the ability to
> specify individual tests to be run (ex: weekly rte_flow testing), this
> script must have the ability to pass in those tests as well. This is a
> change made to support that.
>
> Old syntax:
> "--tags core documentation"
>
> New syntax:
> "--tag core --tag documentation"
>
> Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> ---

1/3, 2/3, and 3/3 applied.

Thanks!

[PATCH v3 2/4] create_new_execution_file_from_tags: add test argument

[ohilyard]

From: Owen Hilyard <ohilyard@iol.unh.edu>

Adds an argument to allow passing in individual test cases to be added
to the resulting DTS execution file. This change was made to facilitate
periodic testing of testcases not currently in CI, such as rte_flow. A
test passed in using this argument will bypass the normal behavior of
only allowing test cases specified in the template execution file. This
behavior because the mapping of DPDK files to tags is not complete, and
to avoid not testing patches, the decision was made to run all tests if
the tests for a patch could not be determined.

Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
---
 tools/create_new_execution_file_from_tags.py | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/create_new_execution_file_from_tags.py b/tools/create_new_execution_file_from_tags.py
index 60ad663..58bcf1a 100755
--- a/tools/create_new_execution_file_from_tags.py
+++ b/tools/create_new_execution_file_from_tags.py
@@ -75,6 +75,8 @@ if __name__ == '__main__':
                         help='What type of testing to create an execution file for')
     parser.add_argument('--tag', type=str, action='append',
                         help='The tags to create an execution file for.')
+    parser.add_argument('--test', type=str, action='append',
+                        help='The tests to run along with the tests required by the provided tags')
 
     args = parser.parse_args()
 
@@ -105,7 +107,11 @@ if __name__ == '__main__':
                 tests_to_run = list(set(test_allowlist).intersection(tests))
             else:
                 tests_to_run = [entry for entry in test_allowlist if entry != '']
-                
+
+            if args.test is not None:
+                for test in args.test:
+                    tests_to_run.append(test)
+
             tests_to_run.sort()
             template_execution_file_parser[execution_plan]['test_suites'] = ", ".join(tests_to_run)
 
-- 
2.30.2

[PATCH v3 3/4] pw_maintainers_cli: module doc comment moved

[ohilyard]

From: Owen Hilyard <ohilyard@iol.unh.edu>

The import statements in the file have been moved below the module
doc comment per PEP 257.

Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
---
 tools/pw_maintainers_cli.py | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/tools/pw_maintainers_cli.py b/tools/pw_maintainers_cli.py
index fd69081..0705eee 100755
--- a/tools/pw_maintainers_cli.py
+++ b/tools/pw_maintainers_cli.py
@@ -3,19 +3,6 @@
 # SPDX-License-Identifier: (BSD-3-Clause AND GPL-2.0-or-later AND MIT)
 # Copyright 2019 Mellanox Technologies, Ltd
 
-import os
-import sys
-import re
-import argparse
-import fnmatch
-
-from requests.exceptions import HTTPError
-
-from git_pw import config
-from git_pw import api
-from git_pw import utils
-from git_pw import patch as git_pw_patch
-
 """
 Description:
 This script uses the git-pw API to retrieve Patchwork's
@@ -53,6 +40,18 @@ Or if you want to use inside other scripts:
     maintainers = maintainers.get_maintainers(tree_url)
 """
 
+import os
+import sys
+import re
+import argparse
+import fnmatch
+
+from requests.exceptions import HTTPError
+
+from git_pw import config
+from git_pw import api
+from git_pw import utils
+from git_pw import patch as git_pw_patch
 
 MAINTAINERS_FILE_PATH = os.environ.get('MAINTAINERS_FILE_PATH')
 if not MAINTAINERS_FILE_PATH:
-- 
2.30.2

RE: [PATCH v3 3/4] pw_maintainers_cli: module doc comment moved

[Ali Alnubani]

> -----Original Message-----
> From: ohilyard@iol.unh.edu <ohilyard@iol.unh.edu>
> Sent: Tuesday, February 1, 2022 11:36 PM
> To: ci@dpdk.org; Ali Alnubani <alialnu@nvidia.com>
> Cc: Owen Hilyard <ohilyard@iol.unh.edu>
> Subject: [PATCH v3 3/4] pw_maintainers_cli: module doc comment moved

This subject line here isn't an instruction of what the commit does. See:
https://doc.dpdk.org/guides/contributing/patches.html#commit-messages-subject-line
"pw_maintainers_cli: move module's doc comment" is a better wording IMO.

Tested-by: Ali Alnubani <alialnu@nvidia.com>

[PATCH v3 4/4] guess_git_tree: fix crash caused by empty diff

[ohilyard]

From: Owen Hilyard <ohilyard@iol.unh.edu>

A sanity check has been added to find_filenames. Occasionally, due to
how the community lab internally handles getting patches from
patchworks, a patch will result in no diff. This patch adds handling for
this case.

Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
---
 tools/pw_maintainers_cli.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/pw_maintainers_cli.py b/tools/pw_maintainers_cli.py
index 0705eee..975b62b 100755
--- a/tools/pw_maintainers_cli.py
+++ b/tools/pw_maintainers_cli.py
@@ -124,6 +124,11 @@ class Diff(object):
             - Moved _filename_re into the method.
             - Reduced newlines.
         """
+        # sanity check diff
+        # for patches without any diff, it will try to run diff.replace
+        # while diff is None. just return an empty list
+        if diff is None:
+            return []
         _filename_re = re.compile(r'^(---|\+\+\+) (\S+)')
         # normalise spaces
         diff = diff.replace('\r', '')
-- 
2.30.2

RE: [PATCH v3 4/4] guess_git_tree: fix crash caused by empty diff

[Ali Alnubani]

> -----Original Message-----
> From: ohilyard@iol.unh.edu <ohilyard@iol.unh.edu>
> Sent: Tuesday, February 1, 2022 11:36 PM
> To: ci@dpdk.org; Ali Alnubani <alialnu@nvidia.com>
> Cc: Owen Hilyard <ohilyard@iol.unh.edu>
> Subject: [PATCH v3 4/4] guess_git_tree: fix crash caused by empty diff

Did you mean "pw_maintainers_cli: fix crash caused by empty diff"?

Tested-by: Ali Alnubani <alialnu@nvidia.com>