CFE-4537, CFE-4538: more unit test coverage

JSON.md

@@ -222,7 +222,7 @@ These are copies of the module directories, where it's more "safe" to do things
 ## All available build steps
 
 The build steps below manipulate the temporary files in the steps directories and write results to the output policy set, in `out/masterfiles`.
-Unless otherwise noted, all steps are run inside the module's folder (`out/steps/...`) with sources / file paths relative to that folder, and targets / destinations mentioned below are relative to the output policy set (`out/masterfiles`, which in the end will be deployed as `/var/cfengine/masterfiles`).
+Unless otherwise noted, all steps are run inside the module's folder (`out/steps/...`) with sources / file paths relative to that folder, and targets / destinations mentioned below are relative to the output policy set (`out/masterfiles`, which in the end will be deployed as `/var/cfengine/masterfiles`). In `cfbs.json`'s `"steps"`, the build step name must be separated from the rest of the build step by a regular space.
 
 * `copy <source> <destination>`
   * Copy a single file or a directory recursively.

cfbs/build.py

@@ -1,18 +1,17 @@
 import os
 import logging as log
+from typing import List, Tuple
 from cfbs.utils import (
     canonify,
     cp,
     deduplicate_def_json,
     find,
-    is_valid_arg_count,
     merge_json,
     mkdir,
     pad_right,
     read_json,
     rm,
     sh,
-    split_command,
     strip_left,
     touch,
     user_error,
@@ -73,8 +72,31 @@ def _generate_augment(module_name, input_data):
     return augment
 
 
+def split_build_step(command) -> Tuple[str, List[str]]:
+    terms = command.split(" ")
+    operation, args = terms[0], terms[1:]
+    return operation, args
+
+
+def step_has_valid_arg_count(args, expected):
+    actual = len(args)
+
+    if type(expected) is int:
+        if actual != expected:
+            return False
+
+    else:
+        # Only other option is a string of 1+, 2+ or similar:
+        assert type(expected) is str and expected.endswith("+")
+        expected = int(expected[0:-1])
+        if actual < expected:
+            return False
+
+    return True
+
+
 def _perform_build_step(module, step, max_length):
-    operation, args = split_command(step)
+    operation, args = split_build_step(step)
     source = module["_directory"]
     counter = module["_counter"]
     destination = "out/masterfiles"
@@ -245,7 +267,7 @@ def perform_build_steps(config) -> int:
     # mini-validation
     for module in config.get("build", []):
         for step in module["steps"]:
-            operation, args = split_command(step)
+            operation, args = split_build_step(step)
 
             if step.split() != [operation] + args:
                 user_error(
@@ -258,7 +280,7 @@ def perform_build_steps(config) -> int:
 
             expected = AVAILABLE_BUILD_STEPS[operation]
             actual = len(args)
-            if not is_valid_arg_count(args, expected):
+            if not step_has_valid_arg_count(args, expected):
                 if type(expected) is int:
                     user_error(
                         "The `%s` build step expects %d arguments, %d were given"

cfbs/utils.py

@@ -5,8 +5,6 @@
 import copy
 import subprocess
 import hashlib
-import logging as log
-from typing import List, Tuple
 import urllib
 import urllib.request  # needed on some platforms
 from collections import OrderedDict
@@ -79,35 +77,12 @@ def cp(src, dst):
     sh("rsync -r %s/ %s" % (src, dst))
 
 
-def pad_left(s, n) -> int:
-    return s if len(s) >= n else " " * (n - len(s)) + s
+def pad_left(s, n):
+    return s.rjust(n)
 
 
-def pad_right(s, n) -> int:
-    return s if len(s) >= n else s + " " * (n - len(s))
-
-
-def split_command(command) -> Tuple[str, List[str]]:
-    terms = command.split(" ")
-    operation, args = terms[0], terms[1:]
-    return operation, args
-
-
-def is_valid_arg_count(args, expected):
-    actual = len(args)
-
-    if type(expected) is int:
-        if actual != expected:
-            return False
-
-    else:
-        # Only other option is a string of 1+, 2+ or similar:
-        assert type(expected) is str and expected.endswith("+")
-        expected = int(expected[0:-1])
-        if actual < expected:
-            return False
-
-    return True
+def pad_right(s, n):
+    return s.ljust(n)
 
 
 def user_error(msg: str):
@@ -137,12 +112,14 @@ def item_index(iterable, item, extra_at_end=True):
 
 
 def strip_right(string, ending):
+    # can be replaced with str.removesuffix from Python 3.9 onwards
     if not string.endswith(ending):
         return string
     return string[0 : -len(ending)]
 
 
 def strip_left(string, beginning):
+    # can be replaced with str.removeprefix from Python 3.9 onwards
     if not string.startswith(beginning):
         return string
     return string[len(beginning) :]
@@ -163,7 +140,7 @@ def save_file(path, data):
         f.write(data)
 
 
-def read_json(path):
+def read_json(path) -> OrderedDict:
     try:
         with open(path, "r") as f:
             return json.loads(f.read(), object_pairs_hook=OrderedDict)
@@ -172,7 +149,7 @@ def read_json(path):
     except NotADirectoryError:
         return None
     except json.decoder.JSONDecodeError as ex:
-        print("Error reading json file {} : {}".format(path, ex))
+        print("Error reading json file '{}': {}".format(path, ex))
         sys.exit(1)
 
 
@@ -272,15 +249,24 @@ def is_cfbs_repo() -> bool:
 
 
 def immediate_subdirectories(path):
-    return [f.name for f in os.scandir(path) if f.is_dir()]
+    l = [f.name for f in os.scandir(path) if f.is_dir()]
+
+    # `os.scandir` returns the entries in arbitrary order, so sort for determinism
+    l = sorted(l)
+
+    return l
 
 
 def immediate_files(path):
-    return [f.name for f in os.scandir(path) if not f.is_dir()]
+    l = [f.name for f in os.scandir(path) if not f.is_dir()]
+
+    # `os.scandir` returns the entries in arbitrary order, so sort for determinism
+    l = sorted(l)
+
+    return l
 
 
 def path_append(dir, subdir):
-    dir = os.path.abspath(os.path.expanduser(dir))
     return dir if not subdir else os.path.join(dir, subdir)
 
 
@@ -296,7 +282,10 @@ def are_paths_equal(path_a, path_b) -> bool:
 
 
 def cfengine_dir(subdir=None):
-    return path_append("~/.cfengine/", subdir)
+    CFENGINE_DIR = "~/.cfengine/"
+    cfengine_dir_abspath = os.path.abspath(os.path.expanduser(CFENGINE_DIR))
+
+    return path_append(cfengine_dir_abspath, subdir)
 
 
 def cfbs_dir(append=None) -> str:

cfbs/validate.py

@@ -1,13 +1,15 @@
 import argparse
-import json
 import sys
 import re
 from collections import OrderedDict
 
-from cfbs.utils import is_valid_arg_count, is_a_commit_hash, split_command, user_error
+from cfbs.utils import (
+    is_a_commit_hash,
+    user_error,
+)
 from cfbs.pretty import TOP_LEVEL_KEYS, MODULE_KEYS
 from cfbs.cfbs_config import CFBSConfig
-from cfbs.build import AVAILABLE_BUILD_STEPS
+from cfbs.build import AVAILABLE_BUILD_STEPS, step_has_valid_arg_count, split_build_step
 
 
 class CFBSValidationError(Exception):
@@ -266,7 +268,7 @@ def validate_steps(name, module):
                 raise CFBSValidationError(
                     name, '"steps" must be a list of non-empty / non-whitespace strings'
                 )
-            operation, args = split_command(step)
+            operation, args = split_build_step(step)
             if not operation in AVAILABLE_BUILD_STEPS:
                 x = ", ".join(AVAILABLE_BUILD_STEPS)
                 raise CFBSValidationError(
@@ -276,7 +278,7 @@ def validate_steps(name, module):
                 )
             expected = AVAILABLE_BUILD_STEPS[operation]
             actual = len(args)
-            if not is_valid_arg_count(args, expected):
+            if not step_has_valid_arg_count(args, expected):
                 if type(expected) is int:
                     raise CFBSValidationError(
                         name,

tests/sample/sample_dir/sample_file_1.txt

@@ -0,0 +1,2 @@
+sample_string
+123

tests/sample/sample_json.json

@@ -0,0 +1,7 @@
+{
+  "a": 1,
+  "b": {
+    "c": "value",
+    "d": [2, "string"]
+  }
+}

tests/shell/035_cfbs_build_compatibility_1.sh

@@ -6,7 +6,7 @@ cd ./tmp/
 rm -rf ./*
 
 # The purpose of this test is to ensure that older CFEngine Build projects
-# still buid in newer versions of cfbs
+# still build in newer versions of cfbs
 
 # The below cfbs.json file was generated using cfbs 3.2.7
 # which is the cfbs version shipped with CFEngine Enterprise 3.21.0

tests/shell/039_add_added_by_field_update_1.sh

@@ -0,0 +1,23 @@
+set -e
+set -x
+cd tests/
+mkdir -p ./tmp/
+cd ./tmp/
+touch cfbs.json && rm cfbs.json
+rm -rf .git
+
+cfbs --non-interactive init
+
+# Ensure adding a module during initialization is treated as adding manually
+cat cfbs.json | grep -F "added_by" | grep -F "cfbs init"
+# TODO: the case of custom non-masterfiles module(s) should also be tested
+
+# Manually adding a module then manually adding its dependency should update the latter's `"added_by"` field in `cfbs.json`
+
+cfbs --non-interactive add package-method-winget
+cat cfbs.json | grep -F "added_by" | grep -F "package-method-winget"
+[ "$(cat cfbs.json | grep -F "added_by" | grep -F "cfbs add" -c)" -eq 1 ]
+
+cfbs --non-interactive add powershell-execution-policy
+! ( cat cfbs.json | grep -F "added_by" | grep -F "package-method-winget" )
+[ "$(cat cfbs.json | grep -F "added_by" | grep -F "cfbs add" -c)" -eq 2 ]

tests/shell/040_add_added_by_field_update_2.sh

@@ -0,0 +1,17 @@
+set -e
+set -x
+cd tests/
+mkdir -p ./tmp/
+cd ./tmp/
+touch cfbs.json && rm cfbs.json
+rm -rf .git
+
+cfbs --non-interactive init
+
+# Manually adding a dependency of a module then manually adding that module should not update the latter's `"added_by"` field in `cfbs.json`
+
+cfbs --non-interactive add powershell-execution-policy
+cat cfbs.json | grep -F "added_by" | grep -F "cfbs add"
+
+cfbs --non-interactive add package-method-winget
+! ( cat cfbs.json | grep -F "added_by" | grep -F "package-method-winget" )

tests/shell/all.sh

@@ -42,5 +42,7 @@ bash tests/shell/035_cfbs_build_compatibility_1.sh
 bash tests/shell/036_cfbs_build_compatibility_2.sh
 bash tests/shell/037_cfbs_validate.sh
 bash tests/shell/038_global_dir.sh
+bash tests/shell/039_add_added_by_field_update_1.sh
+bash tests/shell/040_add_added_by_field_update_2.sh
 
 echo "All cfbs shell tests completed successfully!"