Re: [dpdk-ci] [PATCH] patch parsing: Added library for parsing patch files

[Aaron Conole <aconole@redhat.com>]

ohilyard@iol.unh.edu writes:

> From: Owen Hilyard <ohilyard@iol.unh.edu>
>
> Due to the number of edge cases, there were difficulties doing the parsing using pattern matching, so a dedicated library, whatthepatch, is now used for parsing the patches to ensure correctness.
>
> Also added the default behavior that if no tags are given, create_new_execution_file_from_tags.py will act as though all possible tags were given. This is to allow patches where no tags could be found to still have test coverage.
> ---

Missing sign-off.

Also, please reformat the commit message, preferring 75 column width.
Unlike code, commit logs are very sensitive when viewing history, etc.

>  config/tests_for_tag.cfg                     |  30 +++++
>  requirements.txt                             |   1 +
>  tools/create_new_execution_file_from_tags.py | 114 +++++++++++++++++++
>  tools/patch_parser.py                        |  85 ++++++++++----
>  4 files changed, 205 insertions(+), 25 deletions(-)
>  create mode 100644 config/tests_for_tag.cfg
>  create mode 100644 requirements.txt
>  create mode 100755 tools/create_new_execution_file_from_tags.py
>
> diff --git a/config/tests_for_tag.cfg b/config/tests_for_tag.cfg
> new file mode 100644
> index 0000000..7d95c4a
> --- /dev/null
> +++ b/config/tests_for_tag.cfg
> @@ -0,0 +1,30 @@
> +[functional]
> +core = dynamic_config,
> +       link_status_interrupt,
> +       mac_filter,
> +       mtu_update,
> +       scatter,
> +       stats_checks,
> +       unit_tests_mbuf
> +driver = dynamic_config,
> +       link_status_interrupt,
> +       mac_filter,
> +       mtu_update,
> +       scatter,
> +       stats_checks,
> +       unit_tests_mbuf
> +application = dynamic_config,
> +       link_status_interrupt,
> +       mac_filter,
> +       mtu_update,
> +       scatter,
> +       stats_checks,
> +       unit_tests_mbuf
> +; Nothing in documentation
> +documentation =
> +
> +[performance]
> +core = nic_single_core_perf
> +driver = nic_single_core_perf
> +application = nic_single_core_perf
> +documentation =
> diff --git a/requirements.txt b/requirements.txt
> new file mode 100644
> index 0000000..f20067d
> --- /dev/null
> +++ b/requirements.txt
> @@ -0,0 +1 @@
> +whatthepatch==1.0.2
> \ No newline at end of file
> diff --git a/tools/create_new_execution_file_from_tags.py b/tools/create_new_execution_file_from_tags.py
> new file mode 100755
> index 0000000..d1d4447
> --- /dev/null
> +++ b/tools/create_new_execution_file_from_tags.py
> @@ -0,0 +1,114 @@
> +#!/usr/bin/env python3
> +
> +# BSD LICENSE
> +#
> +# Copyright(c) 2020 Intel Corporation. All rights reserved.
> +# Copyright © 2018[, 2019] The University of New Hampshire. All rights reserved.
> +# All rights reserved.
> +#
> +# Redistribution and use in source and binary forms, with or without
> +# modification, are permitted provided that the following conditions
> +# are met:
> +#
> +#   * Redistributions of source code must retain the above copyright
> +#     notice, this list of conditions and the following disclaimer.
> +#   * Redistributions in binary form must reproduce the above copyright
> +#     notice, this list of conditions and the following disclaimer in
> +#     the documentation and/or other materials provided with the
> +#     distribution.
> +#   * Neither the name of Intel Corporation nor the names of its
> +#     contributors may be used to endorse or promote products derived
> +#     from this software without specific prior written permission.
> +#
> +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> +import sys
> +from enum import Enum
> +
> +import itertools
> +from configparser import ConfigParser
> +from typing import List, Dict, Set
> +import argparse
> +
> +
> +def parse_comma_delimited_list_from_string(mod_str: str) -> List[str]:
> +    return list(map(str.strip, mod_str.split(',')))
> +
> +
> +def map_tags_to_tests(tags: List[str], test_map: Dict[str, List[str]]) -> Set[str]:
> +    """
> +    Returns a list that is the union of all of the map lookups.
> +    """
> +    try:
> +        return set(
> +            filter(lambda test: test != '', set(itertools.chain.from_iterable(map(lambda tag: test_map[tag], tags)))))
> +    except KeyError as e:
> +        print(f'Tag {e} is not present in tests_for_tag.cfg')
> +        exit(1)
> +
> +
> +class TestingType(Enum):
> +    functional = 'functional'
> +    performance = 'performance'
> +
> +    def __str__(self):
> +        return self.value
> +
> +
> +if __name__ == '__main__':
> +    parser = argparse.ArgumentParser(
> +        description='Take a template execution file and add the relevant tests'
> +                    ' for the given tags to it, creating a new file.')
> +    parser.add_argument('config_file_path', help='The path to tests_for_tag.cfg', default='config/tests_for_tag.cfg')
> +    parser.add_argument('template_execution_file', help='The path to the execution file to use as a template')
> +    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.')
> +
> +    args = parser.parse_args()
> +
> +    tag_to_test_map_parser = ConfigParser()
> +    tag_to_test_map_parser.read(args.config_file_path)
> +
> +    template_execution_file_parser = ConfigParser()
> +    template_execution_file_parser.read(args.template_execution_file)
> +
> +    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)
> +
> +    try:
> +        output_file = open(args.output_path, 'x')
> +    except FileExistsError:
> +        output_file = open(args.output_path, 'w')
> +
> +    for execution_plan in template_execution_file_parser:
> +        # The DEFAULT section is always present and contains top-level items, so it needs to be ignored
> +        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))
> +            tests_to_run.sort()
> +            template_execution_file_parser[execution_plan]['test_suites'] = ", ".join(tests_to_run)
> +
> +            if args.testing_type == TestingType.functional:
> +                template_execution_file_parser[execution_plan]['parameters'] += ':func=true:perf=false'
> +            elif args.testing_type == TestingType.performance:
> +                template_execution_file_parser[execution_plan]['parameters'] += ':func=false:perf=true'
> +            else:
> +                # This code should be unreachable, since this is checked at the top of the file
> +                print("Fatal error: testing type is neither performance nor functional", file=sys.stderr)
> +                exit(1)
> +
> +    template_execution_file_parser.write(output_file)
> diff --git a/tools/patch_parser.py b/tools/patch_parser.py
> index 01fc55d..cc91cd2 100755
> --- a/tools/patch_parser.py
> +++ b/tools/patch_parser.py
> @@ -1,27 +1,58 @@
>  #!/usr/bin/env python3
>  
> -import itertools
> -import sys
> +import argparse
>  from configparser import ConfigParser
>  from typing import List, Dict, Set
>  
> +import itertools
> +# BSD LICENSE
> +#
> +# Copyright(c) 2020 Intel Corporation. All rights reserved.
> +# Copyright © 2018[, 2019] The University of New Hampshire. All rights reserved.
> +# All rights reserved.
> +#
> +# Redistribution and use in source and binary forms, with or without
> +# modification, are permitted provided that the following conditions
> +# are met:
> +#
> +#   * Redistributions of source code must retain the above copyright
> +#     notice, this list of conditions and the following disclaimer.
> +#   * Redistributions in binary form must reproduce the above copyright
> +#     notice, this list of conditions and the following disclaimer in
> +#     the documentation and/or other materials provided with the
> +#     distribution.
> +#   * Neither the name of Intel Corporation nor the names of its
> +#     contributors may be used to endorse or promote products derived
> +#     from this software without specific prior written permission.
> +#
> +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> +import sys
>  
> -def get_patch_files(patch_file: str) -> List[str]:
> +try:
> +    import whatthepatch
> +except ImportError:
> +    print("Please install whatthepatch, a patch parsing library", file=sys.stderr)
> +    exit(1)
> +
> +
> +def get_changed_files_in_patch(patch_file: str) -> List[str]:
>      with open(patch_file, 'r') as f:
> -        lines = list(itertools.takewhile(
> -            lambda line: line.strip().endswith('+') or line.strip().endswith('-'),
> -            itertools.dropwhile(
> -                lambda line: not line.strip().startswith("---"),
> -                f.readlines()
> -            )
> -        ))
> -        filenames = map(lambda line: line.strip().split(' ')[0], lines)
> -        # takewhile includes the --- which starts the filenames
> -        return list(filenames)[1:]
> +        filenames = map(lambda diff: diff.header.new_path, whatthepatch.parse_patch(f.read()))
> +        return list(filenames)
>  
>  
>  def get_all_files_from_patches(patch_files: List[str]) -> Set[str]:
> -    return set(itertools.chain.from_iterable(map(get_patch_files, patch_files)))
> +    return set(itertools.chain.from_iterable(map(get_changed_files_in_patch, patch_files)))
>  
>  
>  def parse_comma_delimited_list_from_string(mod_str: str) -> List[str]:
> @@ -47,18 +78,22 @@ def get_tags_for_patches(patch_files: Set[str], dir_attrs: Dict[str, Set[str]])
>      ))
>  
>  
> -if len(sys.argv) < 3:
> -    print("usage: patch_parser.py <path to patch_parser.cfg> <patch file>...")
> -    exit(1)
> +if __name__ == '__main__':
> +    parser = argparse.ArgumentParser(
> +        description='Takes a patch file and a config file and creates a list of tags for that patch')
> +    parser.add_argument('config_file_path', help='The path to patch_parser.cfg', default='config/patch_parser.cfg')
> +    parser.add_argument('patch_file_paths', help='A list of patch files', type=str, metavar='patch file', nargs='+')
> +
> +    args = parser.parse_args()
>  
> -conf_obj = ConfigParser()
> -conf_obj.read(sys.argv[1])
> +    conf_obj = ConfigParser()
> +    conf_obj.read(args.config_file_path)
>  
> -patch_files = get_all_files_from_patches(sys.argv[2:])
> -dir_attrs = get_dictionary_attributes_from_config_file(conf_obj)
> -priority_list = parse_comma_delimited_list_from_string(conf_obj['Priority']['priority_list'])
> +    patch_files = get_all_files_from_patches(args.patch_file_paths)
> +    dir_attrs = get_dictionary_attributes_from_config_file(conf_obj)
> +    priority_list = parse_comma_delimited_list_from_string(conf_obj['Priority']['priority_list'])
>  
> -unordered_tags: Set[str] = get_tags_for_patches(patch_files, dir_attrs)
> -ordered_tags: List[str] = [tag for tag in priority_list if tag in unordered_tags]
> +    unordered_tags: Set[str] = get_tags_for_patches(patch_files, dir_attrs)
> +    ordered_tags: List[str] = [tag for tag in priority_list if tag in unordered_tags]
>  
> -print("\n".join(ordered_tags))
> +    print("\n".join(ordered_tags))