From ab20641054b517bcce9822210b5a8f02205b084e Mon Sep 17 00:00:00 2001
From: Ruslan Mstoi <ruslan.mstoi@intel.com>
Date: Wed, 1 Jul 2020 16:11:08 +0300
Subject: [PATCH] scripts: subfolder_list.py: refactor the code

Refactor subfolder_list.py to enable unit testing. This means split the
implementation into separate logical functions. Additionally, modify the
code to improve PEP 8 compatibility and pylint result.

Signed-off-by: Ruslan Mstoi <ruslan.mstoi@intel.com>
---
 scripts/subfolder_list.py | 114 ++++++++++++++++++++++++--------------
 1 file changed, 72 insertions(+), 42 deletions(-)

diff --git a/scripts/subfolder_list.py b/scripts/subfolder_list.py
index 7e76e86db2e..8f5288928ad 100644
--- a/scripts/subfolder_list.py
+++ b/scripts/subfolder_list.py
@@ -1,29 +1,24 @@
 #!/usr/bin/env python3
 # SPDX-License-Identifier: Apache-2.0
 
+"""Write subfolder list to a file
+
+This script will walk the specified directory and write the file specified with
+the list of all sub-directories found. If the output file already exists, the
+file will only be updated in case sub-directories have been added or removed
+since the previous invocation.
+
+"""
+
 import os
 import argparse
 
 
-def touch(trigger):
-    # If no trigger file is provided then do a return.
-    if trigger is None:
-        return
-
-    if os.path.exists(trigger):
-        os.utime(trigger, None)
-    else:
-        with open(trigger, 'w') as fp:
-            fp.write("")
-
-
-def main():
+def parse_args():
+    """Parse command line arguments and options"""
     parser = argparse.ArgumentParser(
-        description='This script will walk the specified directory and write \
-                     the file specified with the list of all sub-directories \
-                     found. If to the output file already exists, the file \
-                     will only be updated in case sub-directories has been \
-                     added or removed since previous invocation.')
+        description=__doc__,
+        formatter_class=argparse.RawDescriptionHelpFormatter)
 
     parser.add_argument('-d', '--directory', required=True,
                         help='Directory to walk for sub-directory discovery')
@@ -38,47 +33,82 @@ def main():
 
     args = parser.parse_args()
 
+    return args
+
+
+def get_subfolder_list(directory, create_links=None):
+    """Return subfolder list of a directory"""
     dirlist = []
-    if args.create_links is not None:
-        if not os.path.exists(args.create_links):
-            os.makedirs(args.create_links)
-        directory = args.directory
+
+    if create_links is not None:
+        if not os.path.exists(create_links):
+            os.makedirs(create_links)
         symbase = os.path.basename(directory)
-        symlink = args.create_links + os.path.sep + symbase
+        symlink = create_links + os.path.sep + symbase
         if not os.path.exists(symlink):
             os.symlink(directory, symlink)
-        dirlist.extend(symlink)
+        dirlist.append(symlink)
     else:
-        dirlist.extend(args.directory)
-    dirlist.extend(os.linesep)
-    for root, dirs, _ in os.walk(args.directory, topdown=True):
+        dirlist.append(directory)
+
+    for root, dirs, _ in os.walk(directory, topdown=True):
         dirs.sort()
         for subdir in dirs:
-            if args.create_links is not None:
+            if create_links is not None:
                 targetdirectory = os.path.join(root, subdir)
                 reldir = os.path.relpath(targetdirectory, directory)
                 linkname = symbase + '_' + reldir.replace(os.path.sep, '_')
-                symlink = args.create_links + os.path.sep + linkname
+                symlink = create_links + os.path.sep + linkname
                 if not os.path.exists(symlink):
                     os.symlink(targetdirectory, symlink)
-                dirlist.extend(symlink)
+                dirlist.append(symlink)
             else:
-                dirlist.extend(os.path.join(root, subdir))
-            dirlist.extend(os.linesep)
+                dirlist.append(os.path.join(root, subdir))
 
-    new = ''.join(dirlist)
-    existing = ''
+    return dirlist
 
-    if os.path.exists(args.out_file):
-        with open(args.out_file, 'r', encoding="utf-8") as fp:
-            existing = fp.read()
 
-        if new != existing:
-            with open(args.out_file, 'w', encoding="utf-8") as fp:
-                fp.write(new)
+def gen_out_file(out_file, dirs):
+    """Generate file with the list of directories
+
+    File won't be updated if it already exists and has the same content
+
+    """
+    dirs_nl = "\n".join(dirs) + "\n"
+
+    if os.path.exists(out_file):
+        with open(out_file, 'r', encoding="utf-8") as out_file_fo:
+            out_file_dirs = out_file_fo.read()
+
+        if out_file_dirs == dirs_nl:
+            return
+
+    with open(out_file, 'w', encoding="utf-8") as out_file_fo:
+        out_file_fo.writelines(dirs_nl)
+
+
+def touch(trigger):
+    """Touch the trigger file
+
+    If no trigger file is provided then do a return.
+
+    """
+    if trigger is None:
+        return
+
+    if os.path.exists(trigger):
+        os.utime(trigger, None)
     else:
-        with open(args.out_file, 'w', encoding="utf-8") as fp:
-            fp.write(new)
+        with open(trigger, 'w') as trigger_fo:
+            trigger_fo.write("")
+
+
+def main():
+    """Parse command line arguments and take respective actions"""
+    args = parse_args()
+
+    dirs = get_subfolder_list(args.directory, args.create_links)
+    gen_out_file(args.out_file, dirs)
 
     # Always touch trigger file to ensure json files are updated
     touch(args.trigger_file)