diff --git a/.github/workflows/black-format.yml b/.github/workflows/black-format.yml index 64df1eda..b43fd282 100644 --- a/.github/workflows/black-format.yml +++ b/.github/workflows/black-format.yml @@ -1,11 +1,11 @@ name: Automatically format with Black and submit PR on: - push: - branches: - - master + push: + branches: + - master - workflow_dispatch: + workflow_dispatch: jobs: format: @@ -14,57 +14,57 @@ jobs: contents: write pull-requests: write steps: - - name: Checks-out repository - uses: actions/checkout@v4 - - name: Set up Python 3.12 - uses: actions/setup-python@v5 - with: - python-version: "3.12" - - name: Install black - run: | - python -m pip install --upgrade pip - python -m pip install black - - name: Reformat with black - run: | - shopt -s globstar - black cfbs/*.py cfbs/**/*.py tests/*.py tests/**/*.py > black_output.txt 2>&1 - - name: Check if there are changes - run: | - git diff --exit-code || touch git_diff_exists - if [ -f git_diff_exists ]; then echo "Changes need to be commited"; else echo "No changes to commit"; fi - - name: Create commit message - if: hashFiles('git_diff_exists') != '' - run: | - echo "Reformatted python code using Black formatter" >> commit_message.txt - echo "" >> commit_message.txt - echo "Output from black:" >> commit_message.txt - echo "" >> commit_message.txt - echo '```' >> commit_message.txt - cat black_output.txt >> commit_message.txt - echo '```' >> commit_message.txt - - name: Commit changes - if: hashFiles('git_diff_exists') != '' - run: | - git config user.name 'GitHub' - git config user.email '' - shopt -s globstar - git add cfbs/*.py cfbs/**/*.py tests/*.py tests/**/*.py - git commit -F commit_message.txt - - id: commit-message-from-file - name: Parse commit message from file into variable - if: hashFiles('git_diff_exists') != '' - run: | - body=$(cat commit_message.txt) - body="${body//$'\n'/'%0A'}" - echo ::set-output name=body::$body - - name: Create Pull Request - if: hashFiles('git_diff_exists') != '' - uses: cfengine/create-pull-request@v6 - with: - title: Reformatted python code using Black formatter - body: ${{ steps.commit-message-from-file.outputs.body }} - reviewers: | - olehermanse - larsewi - vpodzime - branch: formatting-action + - name: Checks-out repository + uses: actions/checkout@v4 + - name: Set up Python 3.12 + uses: actions/setup-python@v5 + with: + python-version: "3.12" + - name: Install black + run: | + python -m pip install --upgrade pip + python -m pip install black + - name: Reformat with black + run: | + shopt -s globstar + black cfbs/*.py cfbs/**/*.py tests/*.py tests/**/*.py > black_output.txt 2>&1 + - name: Check if there are changes + run: | + git diff --exit-code || touch git_diff_exists + if [ -f git_diff_exists ]; then echo "Changes need to be commited"; else echo "No changes to commit"; fi + - name: Create commit message + if: hashFiles('git_diff_exists') != '' + run: | + echo "Reformatted python code using Black formatter" >> commit_message.txt + echo "" >> commit_message.txt + echo "Output from black:" >> commit_message.txt + echo "" >> commit_message.txt + echo '```' >> commit_message.txt + cat black_output.txt >> commit_message.txt + echo '```' >> commit_message.txt + - name: Commit changes + if: hashFiles('git_diff_exists') != '' + run: | + git config user.name 'GitHub' + git config user.email '' + shopt -s globstar + git add cfbs/*.py cfbs/**/*.py tests/*.py tests/**/*.py + git commit -F commit_message.txt + - id: commit-message-from-file + name: Parse commit message from file into variable + if: hashFiles('git_diff_exists') != '' + run: | + body=$(cat commit_message.txt) + body="${body//$'\n'/'%0A'}" + echo ::set-output name=body::$body + - name: Create Pull Request + if: hashFiles('git_diff_exists') != '' + uses: cfengine/create-pull-request@v6 + with: + title: Reformatted python code using Black formatter + body: ${{ steps.commit-message-from-file.outputs.body }} + reviewers: | + olehermanse + larsewi + vpodzime + branch: formatting-action diff --git a/.github/workflows/black.yml b/.github/workflows/black.yml index f5296bc5..b2ee2f38 100644 --- a/.github/workflows/black.yml +++ b/.github/workflows/black.yml @@ -5,9 +5,9 @@ name: Black on: push: - branches: [ master ] + branches: [master] pull_request: - branches: [ master ] + branches: [master] jobs: check: @@ -18,17 +18,17 @@ jobs: python-version: ["3.12"] steps: - - uses: actions/checkout@v4 - - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v5 - with: - python-version: ${{ matrix.python-version }} - - name: Install dependencies - run: | - python -m pip install --upgrade pip - python -m pip install black - if [ -f requirements.txt ]; then pip install -r requirements.txt; fi - - name: Check formatting with black - run: | - shopt -s globstar - black --check cfbs/*.py cfbs/**/*.py tests/*.py tests/**/*.py + - uses: actions/checkout@v4 + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python-version }} + - name: Install dependencies + run: | + python -m pip install --upgrade pip + python -m pip install black + if [ -f requirements.txt ]; then pip install -r requirements.txt; fi + - name: Check formatting with black + run: | + shopt -s globstar + black --check cfbs/*.py cfbs/**/*.py tests/*.py tests/**/*.py diff --git a/.github/workflows/python-publish.yml b/.github/workflows/python-publish.yml index a6b23211..7cffb8d0 100644 --- a/.github/workflows/python-publish.yml +++ b/.github/workflows/python-publish.yml @@ -9,23 +9,22 @@ on: jobs: deploy: - runs-on: ubuntu-24.04 steps: - - uses: actions/checkout@v4 - - name: Set up Python - uses: actions/setup-python@v5 - with: - python-version: '3.x' - - name: Install dependencies - run: | - python -m pip install --upgrade pip - pip install setuptools wheel twine - - name: Build and publish - env: - TWINE_USERNAME: ${{ secrets.PYPI_USERNAME }} - TWINE_PASSWORD: ${{ secrets.PYPI_PASSWORD }} - run: | - python setup.py sdist bdist_wheel - twine upload dist/* + - uses: actions/checkout@v4 + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: "3.x" + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install setuptools wheel twine + - name: Build and publish + env: + TWINE_USERNAME: ${{ secrets.PYPI_USERNAME }} + TWINE_PASSWORD: ${{ secrets.PYPI_PASSWORD }} + run: | + python setup.py sdist bdist_wheel + twine upload dist/* diff --git a/.github/workflows/python-tests.yml b/.github/workflows/python-tests.yml index 602bd00e..15820549 100644 --- a/.github/workflows/python-tests.yml +++ b/.github/workflows/python-tests.yml @@ -5,9 +5,9 @@ name: Python tests on: push: - branches: [ master ] + branches: [master] pull_request: - branches: [ master ] + branches: [master] jobs: test: @@ -18,34 +18,37 @@ jobs: python-version: ["3.8", "3.9", "3.10", "3.11", "3.12", "3.13"] steps: - - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v5 - with: - python-version: ${{ matrix.python-version }} - - name: Install dependencies - run: | - python -m pip install --upgrade pip - python -m pip install flake8 pytest setuptools wheel - if [ -f requirements.txt ]; then pip install -r requirements.txt; fi - - name: Lint with flake8 - run: | - # stop the build if there are Python syntax errors or undefined names - flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics - # exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide - flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics - - name: Test with pytest - run: | - pytest - - name: Install - run: | - python setup.py sdist bdist_wheel - pip install dist/cfbs-*.whl - - name: Run bash tests - run: | - UNSAFE_TESTS=1 bash tests/shell/all.sh + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python-version }} + - name: Install dependencies + run: | + python -m pip install --upgrade pip + python -m pip install flake8 pyright pyflakes pytest setuptools wheel + if [ -f requirements.txt ]; then pip install -r requirements.txt; fi + - name: Lint with flake8 + run: | + flake8 . --ignore=E203,W503,E722,E731 --max-complexity=100 --max-line-length=160 + - name: Lint with pyright (type checking) + run: | + pyright cfbs + - name: Lint with pyflakes + run: | + pyflakes cfbs + - name: Test with pytest + run: | + pytest + - name: Install + run: | + python setup.py sdist bdist_wheel + pip install dist/cfbs-*.whl + - name: Run bash tests + run: | + UNSAFE_TESTS=1 bash tests/shell/all.sh test-legacy: runs-on: ubuntu-24.04 env: @@ -57,26 +60,26 @@ jobs: python-version: ["3.5.10", "3.6.15", "3.7.10"] steps: - - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - name: Set up legacy Python ${{ matrix.python-version }} - uses: ./.github/actions/set-up-legacy-python - with: - python-version: ${{ matrix.python-version }} - - name: Lint with flake8 - run: | - # stop the build if there are Python syntax errors or undefined names - flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics - # exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide - flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics - - name: Test with pytest - run: | - pytest - - name: Install - run: | - python setup.py sdist bdist_wheel - pip install dist/cfbs-*.whl - - name: Run bash tests - run: | - UNSAFE_TESTS=1 bash tests/shell/all.sh + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - name: Set up legacy Python ${{ matrix.python-version }} + uses: ./.github/actions/set-up-legacy-python + with: + python-version: ${{ matrix.python-version }} + - name: Lint with flake8 + run: | + # stop the build if there are Python syntax errors or undefined names + flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics + # exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide + flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics + - name: Test with pytest + run: | + pytest + - name: Install + run: | + python setup.py sdist bdist_wheel + pip install dist/cfbs-*.whl + - name: Run bash tests + run: | + UNSAFE_TESTS=1 bash tests/shell/all.sh diff --git a/.github/workflows/python-validate-test.yml b/.github/workflows/python-validate-test.yml index 0de137b4..a03fdca2 100644 --- a/.github/workflows/python-validate-test.yml +++ b/.github/workflows/python-validate-test.yml @@ -5,9 +5,9 @@ name: Python validate test on: push: - branches: [ master ] + branches: [master] pull_request: - branches: [ master ] + branches: [master] jobs: test-legacy: @@ -21,17 +21,17 @@ jobs: python-version: ["3.5.10"] steps: - - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - name: Set up legacy Python ${{ matrix.python-version }} - uses: ./.github/actions/set-up-legacy-python - with: - python-version: ${{ matrix.python-version }} - - name: Install - run: | - python setup.py sdist bdist_wheel - pip install dist/cfbs-*.whl - - name: Run bash tests - run: | - bash tests/shell/validate.sh + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - name: Set up legacy Python ${{ matrix.python-version }} + uses: ./.github/actions/set-up-legacy-python + with: + python-version: ${{ matrix.python-version }} + - name: Install + run: | + python setup.py sdist bdist_wheel + pip install dist/cfbs-*.whl + - name: Run bash tests + run: | + bash tests/shell/validate.sh diff --git a/cfbs/analyze.py b/cfbs/analyze.py index b6f63a45..43588049 100644 --- a/cfbs/analyze.py +++ b/cfbs/analyze.py @@ -87,11 +87,11 @@ def checksums_files( if contains_ignored_components(full_relpath, ignored_path_components): continue - if not file_checksum in checksums_dict["checksums"]: + if file_checksum not in checksums_dict["checksums"]: checksums_dict["checksums"][file_checksum] = set() checksums_dict["checksums"][file_checksum].add(tarball_relpath) - if not tarball_relpath in files_dict["files"]: + if tarball_relpath not in files_dict["files"]: files_dict["files"][tarball_relpath] = set() files_dict["files"][tarball_relpath].add(file_checksum) @@ -108,7 +108,10 @@ def mpf_vcf_dicts(offline=False): RI_SUBDIRS = "downloads/github.com/" + REPO_OWNERNAME + "/archive/refs/tags/" if offline: - ERROR_MESSAGE = "Masterfiles Policy Framework release information not found. Provide the release information, for example by running 'cfbs analyze' without '--offline'." + ERROR_MESSAGE = ( + "Masterfiles Policy Framework release information not found. " + + "Provide the release information, for example by running 'cfbs analyze' without '--offline'." + ) cfbs_ri_dir = os.path.join(cfbs_dir(), RI_SUBDIRS) if not os.path.exists(cfbs_ri_dir): @@ -133,7 +136,7 @@ def mpf_vcf_dicts(offline=False): try: latest_release_data = get_json(LATEST_RELEASE_API_URL) - except FetchError as e: + except FetchError: raise GenericExitError( "Downloading CFEngine release information failed - check your Wi-Fi / network settings." ) @@ -175,13 +178,16 @@ def mpf_vcf_dicts(offline=False): mpf_files_json_path = os.path.join(mpf_vcf_path, "files.json") mpf_versions_dict = read_json(mpf_versions_json_path) + assert mpf_versions_dict is not None mpf_versions_dict = mpf_versions_dict["versions"] mpf_checksums_dict = read_json(mpf_checkfiles_json_path) + assert mpf_checksums_dict is not None mpf_checksums_dict = mpf_checksums_dict["checksums"] mpf_files_dict = read_json(mpf_files_json_path) + assert mpf_files_dict is not None mpf_files_dict = mpf_files_dict["files"] return mpf_versions_dict, mpf_checksums_dict, mpf_files_dict @@ -201,10 +207,10 @@ def filepaths_display(filepaths): print("└──", filepaths[-1]) -def list_or_single(l): - if len(l) == 1: - return l[0] - return l +def list_or_single(elements): + if len(elements) == 1: + return elements[0] + return elements def filepaths_display_moved(filepaths): @@ -272,7 +278,9 @@ def most_common_version(self): return highest_version(versions_with_highest_count) def sorted_list(self): - """Returns a sorted list of key-value pairs `(version, count)`. The sorting is in descending order. In case of a count tie, the higher version's pair is considered greater.""" + """Returns a sorted list of key-value pairs `(version, count)`. + The sorting is in descending order. In case of a count tie, + the higher version's pair is considered greater.""" return sorted( self._versions_counts.items(), key=lambda item: (item[1], version_as_comparable_list(item[0])), @@ -350,6 +358,7 @@ def __init__(self, reference_version): self.different_moved_or_renamed = [] self.not_from_any = [] + @staticmethod def _denormalize_origin(origin, is_parentpath, masterfiles_dir): return [ (mpf_denormalized_path(filepath, is_parentpath, masterfiles_dir), versions) @@ -499,9 +508,15 @@ def analyze_policyset( ignored_path_components=None, offline=False, ): - """`path` should be either a masterfiles-path (containing masterfiles files directly), or a parent-path (containing `masterfiles_dir` and "modules" folders). `is_parentpath` should specify which of the two it is. - - The analysis ignores policyset (not MPF release information) files whose filepaths contain any of the path components specified in `ignored_path_components`. Components in `ignored_path_components` should end with a `/` if the component represents a directory (also on operating systems using a different separator e.g. a backslash), and should not end with a `/` if it represents a file. + """`path` should be either a masterfiles-path (containing masterfiles files directly), + or a parent-path (containing `masterfiles_dir` and "modules" folders). `is_parentpath` + should specify which of the two it is. + + The analysis ignores policyset (not MPF release information) files whose filepaths + contain any of the path components specified in `ignored_path_components`. + Components in `ignored_path_components` should end with a `/` if the component + represents a directory (also on operating systems using a different separator + e.g. a backslash), and should not end with a `/` if it represents a file. """ if ignored_path_components is None: ignored_path_components = DEFAULT_IGNORED_PATH_COMPONENTS @@ -574,7 +589,8 @@ def analyze_policyset( if reference_version is None: reference_version = versions_data.version_counter.most_common_version() - # if not a single file in the analyzed policyset has an MPF-known (checksum, filepath), and a specific `reference_version` was not given, `reference_version` will still be `None` + # if not a single file in the analyzed policyset has an MPF-known (checksum, filepath), + # and a specific `reference_version` was not given, `reference_version` will still be `None` if reference_version is None: # try to detect whether the user provided a wrong policy set path # gather all possible policy set paths, by detecting promises.cf or update.cf diff --git a/cfbs/cfbs_config.py b/cfbs/cfbs_config.py index f9750891..32b5001d 100644 --- a/cfbs/cfbs_config.py +++ b/cfbs/cfbs_config.py @@ -33,7 +33,7 @@ def __init__(self, r): def _has_autorun_tag(filename): assert os.path.isfile(filename) content = read_file(filename) - + assert content is not None return ( "meta:" in content and "tags" in content @@ -99,8 +99,10 @@ def add_with_dependencies( if type(module) is str: module_str = module module = (remote_config or self).get_module_for_build(module, str_added_by) + if not module: + raise GenericExitError("Module '%s' not found" % module_str) if not module: - raise GenericExitError("Module '%s' not found" % module_str) + raise GenericExitError("Module '%s' not found" % str(module)) assert "name" in module name = module["name"] assert "steps" in module @@ -111,6 +113,7 @@ def add_with_dependencies( if "dependencies" in module: for dep in module["dependencies"]: self.add_with_dependencies(dep, remote_config, name) + assert self._data is not None if "build" not in self._data: self._data["build"] = [] self._data["build"].append(module) @@ -298,7 +301,9 @@ def _handle_local_module(self, module): pattern = "%s/**/*.cf" % name policy_files = glob.glob(pattern, recursive=True) - modules_available = [m.get("name", "") for m in self.get("build", [])] + modules_in_build_key = self.get("build", []) + assert type(modules_in_build_key) is list + modules_available = [m.get("name", "") for m in modules_in_build_key] is_autorun_enabled = any( m in modules_available for m in ["autorun", "./def.json"] ) # only a heuristic @@ -325,7 +330,7 @@ def _add_without_dependencies(self, modules): assert name not in (m["name"] for m in self["build"]) if "subdirectory" in module and module["subdirectory"] == "": del module["subdirectory"] - if self.index.custom_index != None: + if self.index.custom_index is not None: module["index"] = self.index.custom_index self["build"].append(module) self._handle_local_module(module) @@ -358,7 +363,7 @@ def _add_modules( # Filter modules which are already added: to_add = self._filter_modules_to_add(modules) if not to_add: - return # Everything already added + return 0 # Everything already added # Convert names to objects: modules_to_add = [index.get_module_object(m, added_by[m.name]) for m in to_add] @@ -374,17 +379,20 @@ def _add_modules( self._add_without_dependencies(dependencies) self._add_without_dependencies(modules_to_add) + return 0 def add_command( self, to_add: list, added_by="cfbs add", checksum=None, - ) -> int: + ) -> Result: if not to_add: raise GenericExitError("Must specify at least one module to add") - before = {m["name"] for m in self.get("build", [])} + modules_in_build_key = self.get("build", []) + assert type(modules_in_build_key) is list + before = {m["name"] for m in modules_in_build_key} if to_add[0].startswith(SUPPORTED_URI_SCHEMES): self._add_using_url( @@ -414,6 +422,7 @@ def add_command( files.append(name) module = self.get_module_from_build(name) + assert module is not None, "All added modules should exist in build" input_path = os.path.join(".", name, "input.json") if "input" in module: if os.path.isfile(input_path): diff --git a/cfbs/cfbs_json.py b/cfbs/cfbs_json.py index c345ebfd..7f34bde7 100644 --- a/cfbs/cfbs_json.py +++ b/cfbs/cfbs_json.py @@ -71,6 +71,7 @@ def raw_data(self): def _find_all_module_objects(self): data = self.raw_data + assert data is not None modules = [] if "index" in data and type(data["index"]) in (dict, OrderedDict): modules += data["index"].values() @@ -118,6 +119,7 @@ def get(self, key, default=None): def __getitem__(self, key): assert key != "index" + assert self._data is not None return self._data[key] def __contains__(self, key): @@ -125,6 +127,7 @@ def __contains__(self, key): def get_provides(self, added_by="cfbs add"): modules = OrderedDict() + assert self._data is not None if "provides" not in self._data: raise GenericExitError( "missing required key 'provides' in module definition: %s" @@ -138,6 +141,7 @@ def get_provides(self, added_by="cfbs add"): return modules def get_module_for_build(self, name, added_by="cfbs add"): + assert self._data is not None if "provides" in self._data and name in self._data["provides"]: module = self._data["provides"][name] return _construct_provided_module( diff --git a/cfbs/commands.py b/cfbs/commands.py index f32b9baa..e00b56b8 100644 --- a/cfbs/commands.py +++ b/cfbs/commands.py @@ -9,6 +9,7 @@ import logging as log import json import functools +from typing import Union from collections import OrderedDict from cfbs.analyze import analyze_policyset from cfbs.args import get_args @@ -17,7 +18,6 @@ from cfbs.updates import ModuleUpdates, update_module from cfbs.utils import ( FetchError, - cfbs_dir, cfbs_filename, is_cfbs_repo, read_json, @@ -280,9 +280,13 @@ def init_command(index=None, masterfiles=None, non_interactive=False) -> int: log.debug("Current commit for masterfiles branch %s is %s" % (branch, commit)) to_add = ["%s@%s" % (remote, commit), "masterfiles"] if to_add: - ret = add_command(to_add, added_by="cfbs init") - if ret != 0: - return ret + result = add_command(to_add, added_by="cfbs init") + assert not isinstance( + result, Result + ), "Our git decorators are confusing the type checkers" + if result != 0: + return result + # TODO: Do we need to make commits here? return 0 @@ -297,6 +301,7 @@ def status_command() -> int: print("Description: %s" % config["description"]) print("File: %s" % cfbs_filename()) if "index" in config: + assert config.raw_data is not None index = config.raw_data["index"] if type(index) is str: @@ -384,7 +389,7 @@ def add_command( to_add: list, added_by="cfbs add", checksum=None, -) -> int: +) -> Union[Result, int]: config = CFBSConfig.get_instance() config.warn_about_unknown_keys() r = config.add_command(to_add, added_by, checksum) @@ -397,7 +402,7 @@ def add_command( def remove_command(to_remove: list): config = CFBSConfig.get_instance() config.warn_about_unknown_keys() - if not "build" in config: + if "build" not in config: raise GenericExitError( 'Cannot remove any modules because the "build" key is missing from cfbs.json' ) @@ -418,7 +423,7 @@ def reduce_dependencies(a, b): return functools.reduce(reduce_dependencies, modules) - def _get_module_by_name(name) -> dict: + def _get_module_by_name(name) -> Union[dict, None]: if not name.startswith("./") and name.endswith(".cf") and os.path.exists(name): name = "./" + name @@ -525,7 +530,7 @@ def _someone_needs_me(this) -> bool: if ("added_by" not in this) or is_module_added_manually(this["added_by"]): return True for other in modules: - if not "dependencies" in other: + if "dependencies" not in other: continue if this["name"] in other["dependencies"]: return _someone_needs_me(other) @@ -576,9 +581,13 @@ def update_command(to_update) -> Result: updated = [] module_updates = ModuleUpdates(config) + index = None for update in to_update: old_module = config.get_module_from_build(update.name) + assert ( + old_module is not None + ), 'We\'ve already checked that modules are in config["build"]' custom_index = old_module is not None and "index" in old_module index = Index(old_module["index"]) if custom_index else config.index @@ -663,6 +672,7 @@ def update_command(to_update) -> Result: updated.append(update) if module_updates.new_deps: + assert index is not None objects = [ index.get_module_object(d, module_updates.new_deps_added_by[d]) for d in module_updates.new_deps @@ -702,7 +712,6 @@ def _download_dependencies( print("\nModules:") counter = 1 max_length = config.longest_module_key_length("name") - downloads = os.path.join(cfbs_dir(), "downloads") for module in config.get("build", []): name = module["name"] if name.startswith("./"): @@ -748,7 +757,7 @@ def _download_dependencies( else: try: versions = get_json(_VERSION_INDEX) - except FetchError as e: + except FetchError: raise GenericExitError( "Downloading CFEngine Build Module Index failed - check your Wi-Fi / network settings." ) @@ -779,7 +788,7 @@ def _download_dependencies( @cfbs_command("download") -def download_command(force, ignore_versions=False): +def download_command(force, ignore_versions=False) -> int: config = CFBSConfig.get_instance() r = validate_config(config) if r != 0: @@ -789,6 +798,7 @@ def download_command(force, ignore_versions=False): + "\nIf not fixed, these errors will cause your project to not build in future cfbs versions." ) _download_dependencies(config, redownload=force, ignore_versions=ignore_versions) + return 0 @cfbs_command("build") @@ -876,7 +886,8 @@ def info_command(modules): config.warn_about_unknown_keys() index = config.index - build = config.get("build", {}) + build = config.get("build", []) + assert isinstance(build, list) alias = None @@ -1057,7 +1068,7 @@ def _compare_list(a, b): if len(a) != len(b): return False for x, y in zip(a, b): - if type(x) != type(y): + if type(x) is not type(y): return False if isinstance(x, dict): if not _compare_dict(x, y): @@ -1066,8 +1077,8 @@ def _compare_list(a, b): if not _compare_list(x, y): return False else: - assert isinstance( - x, (int, float, str, bool, None) + assert x is None or isinstance( + x, (int, float, str, bool) ), "Illegal value type" if x != y: return False @@ -1138,3 +1149,4 @@ def generate_release_information_command( omit_download=False, check=False, min_version=None ): generate_release_information(omit_download, check, min_version) + return 0 diff --git a/cfbs/git.py b/cfbs/git.py index f0d7fccf..98d141e4 100644 --- a/cfbs/git.py +++ b/cfbs/git.py @@ -12,6 +12,7 @@ import itertools import tempfile from subprocess import check_call, check_output, run, PIPE, DEVNULL, CalledProcessError +from typing import Iterable, Union from cfbs.utils import are_paths_equal @@ -115,7 +116,11 @@ def git_init(user_name=None, user_email=None, description=None, initial_branch=" def git_commit( - commit_msg, edit_commit_msg=False, user_name=None, user_email=None, scope="all" + commit_msg, + edit_commit_msg=False, + user_name=None, + user_email=None, + scope: Union[str, Iterable[str]] = "all", ): """Create a commit in the CWD Git repository diff --git a/cfbs/git_magic.py b/cfbs/git_magic.py index 212f8012..9589a96e 100644 --- a/cfbs/git_magic.py +++ b/cfbs/git_magic.py @@ -4,6 +4,7 @@ do prompts, etc. """ +from typing import Iterable, Union from cfbs.prompts import YES_NO_CHOICES, prompt_user from cfbs.cfbs_config import CFBSConfig, CFBSReturnWithoutCommit from cfbs.git import ( @@ -22,7 +23,9 @@ first_commit = True -def git_commit_maybe_prompt(commit_msg, non_interactive, scope="all"): +def git_commit_maybe_prompt( + commit_msg, non_interactive, scope: Union[str, Iterable[str]] = "all" +): edit_commit_msg = False args = get_args() @@ -136,4 +139,6 @@ def decorated_fn(*args, **kwargs): return decorator -commit_after_command = partial(with_git_commit, (0,), ("cfbs.json",), failed_return=1) +commit_after_command = partial( + with_git_commit, (0,), ("cfbs.json",), failed_return=True +) diff --git a/cfbs/index.py b/cfbs/index.py index 3724f573..f4a49851 100644 --- a/cfbs/index.py +++ b/cfbs/index.py @@ -1,5 +1,7 @@ -import sys, os +import sys +import os from collections import OrderedDict +from typing import Union from cfbs.module import Module from cfbs.utils import FetchError, get_or_read_json, GenericExitError, get_json @@ -89,7 +91,7 @@ def _expand_index(self): try: self._data = get_or_read_json(index) - except FetchError as e: + except FetchError: raise GenericExitError( "Downloading index '%s' failed - check your Wi-Fi / network settings." % index @@ -104,10 +106,11 @@ def _expand_index(self): def data(self) -> dict: if not self._data: self._expand_index() + assert self._data, "_expand_index() should have set _data" return self._data @property - def custom_index(self) -> str: + def custom_index(self) -> Union[str, None]: # Index can be initialized with a dict or OrderedDict instead of a url string # in which case there would not be an index url to check against the default if type(self._unexpanded) is str and self._unexpanded != _DEFAULT_INDEX: @@ -129,7 +132,7 @@ def exists(self, module): return name in self try: versions = get_json(_VERSION_INDEX) - except FetchError as e: + except FetchError: raise GenericExitError( "Downloading CFEngine Build Module Index failed - check your Wi-Fi / network settings." ) @@ -176,7 +179,7 @@ def get_module_object(self, module, added_by=None): if version: try: versions = get_json(_VERSION_INDEX) - except FetchError as e: + except FetchError: raise GenericExitError( "Downloading CFEngine Build Module Index failed - check your Wi-Fi / network settings." ) @@ -186,6 +189,8 @@ def get_module_object(self, module, added_by=None): } object.update(specifics) object["version"] = version + + assert object is not None module.update(object) if added_by: module["added_by"] = added_by diff --git a/cfbs/internal_file_management.py b/cfbs/internal_file_management.py index 05a64ece..ae8c9c1d 100644 --- a/cfbs/internal_file_management.py +++ b/cfbs/internal_file_management.py @@ -211,7 +211,6 @@ def fetch_archive( else: raise GenericExitError("Unsupported archive type: '%s'" % url) - archive_name = strip_right(archive_filename, archive_type) downloads = os.path.join(cfbs_dir(), "downloads") archive_dir = os.path.join(downloads, archive_dirname) @@ -226,6 +225,7 @@ def fetch_archive( content_dir = os.path.join(downloads, archive_dir, archive_checksum) if extract_to_directory: + assert directory is not None content_dir = directory index_path = os.path.join(content_dir, "cfbs.json") if with_index and os.path.exists(index_path): diff --git a/cfbs/main.py b/cfbs/main.py index bb7026bf..19154580 100644 --- a/cfbs/main.py +++ b/cfbs/main.py @@ -5,7 +5,9 @@ import logging as log import sys +from typing import Union +from cfbs.result import Result from cfbs.validate import CFBSValidationError from cfbs.version import string as version from cfbs.utils import GenericExitError, is_cfbs_repo, ProgrammerError @@ -38,7 +40,7 @@ def does_log_info(level): return level == "info" or level == "debug" -def _main() -> int: +def _main() -> Union[int, Result]: """Actual body of main function. Mainly for getting command line arguments and calling the appropriate @@ -245,7 +247,11 @@ def main() -> int: The only thing we want to do here is call _main() and handle exceptions (errors). """ try: - return _main() + r = _main() + # TODO: I'm not exactly sure when the commands + # would actually return result and what that really means. + assert type(r) is int + return r except CFBSValidationError as e: print("Error: " + str(e)) except GenericExitError as e: diff --git a/cfbs/man_generator.py b/cfbs/man_generator.py index 97318ebe..0f15c61d 100644 --- a/cfbs/man_generator.py +++ b/cfbs/man_generator.py @@ -3,7 +3,7 @@ from args import get_arg_parser try: - from build_manpages.manpage import Manpage + from build_manpages.manpage import Manpage # type: ignore except ImportError: raise GenericExitError( "Missing dependency, install from PyPI: 'pip install argparse-manpage setuptools'" @@ -13,7 +13,11 @@ def generate_man_page(): manpage = Manpage(get_arg_parser()) manpage.manual = "CFEngine Build System manual" - manpage.description = "combines multiple modules into 1 policy set to deploy on your infrastructure. Modules can be custom promise types, JSON files which enable certain functionality, or reusable CFEngine policy. The modules you use can be written by the CFEngine team, others in the community, your colleagues, or yourself." + manpage.description = ( + "combines multiple modules into 1 policy set to deploy on your infrastructure. " + + "Modules can be custom promise types, JSON files which enable certain functionality, or reusable CFEngine policy. " + + "The modules you use can be written by the CFEngine team, others in the community, your colleagues, or yourself." + ) body = ( str(manpage) + """ diff --git a/cfbs/masterfiles/analyze.py b/cfbs/masterfiles/analyze.py index 71698806..106852b4 100644 --- a/cfbs/masterfiles/analyze.py +++ b/cfbs/masterfiles/analyze.py @@ -25,15 +25,15 @@ def versions_checksums_files( versions_dict["versions"][version] = {} versions_dict["versions"][version][tarball_relpath] = file_checksum - if not file_checksum in checksums_dict["checksums"]: + if file_checksum not in checksums_dict["checksums"]: checksums_dict["checksums"][file_checksum] = {} - if not tarball_relpath in checksums_dict["checksums"][file_checksum]: + if tarball_relpath not in checksums_dict["checksums"][file_checksum]: checksums_dict["checksums"][file_checksum][tarball_relpath] = [] checksums_dict["checksums"][file_checksum][tarball_relpath].append(version) - if not tarball_relpath in files_dict["files"]: + if tarball_relpath not in files_dict["files"]: files_dict["files"][tarball_relpath] = {} - if not file_checksum in files_dict["files"][tarball_relpath]: + if file_checksum not in files_dict["files"][tarball_relpath]: files_dict["files"][tarball_relpath][file_checksum] = [] files_dict["files"][tarball_relpath][file_checksum].append(version) @@ -108,12 +108,14 @@ def version_as_comparable_list(version: str): version += "|0.0" version = version.replace("x", "99999").replace("-", "|1.").replace("b", "|-1.") versionpair = version.split("|") - versionlist = [versionpair[0].split("."), versionpair[1].split(".")] + versions_str = [versionpair[0].split("."), versionpair[1].split(".")] - versionlist[0] = [int(s) for s in versionlist[0]] - versionlist[1] = [int(s) for s in versionlist[1]] + versions_int = [ + [int(s) for s in versions_str[0]], + [int(s) for s in versions_str[1]], + ] - return versionlist + return versions_int def version_as_comparable_list_negated(version): diff --git a/cfbs/masterfiles/check_download_matches_git.py b/cfbs/masterfiles/check_download_matches_git.py index 029458df..2f1e8278 100644 --- a/cfbs/masterfiles/check_download_matches_git.py +++ b/cfbs/masterfiles/check_download_matches_git.py @@ -1,3 +1,4 @@ +import os from collections import OrderedDict from cfbs.masterfiles.analyze import version_as_comparable_list @@ -11,10 +12,15 @@ def check_download_matches_git(versions): Generates a `differences-*.txt` file for each version. """ + assert os.path.isfile("versions.json") + assert os.path.isfile("versions-git.json") download_versions_dict = read_json("versions.json") git_versions_dict = read_json("versions-git.json") + assert download_versions_dict is not None + assert git_versions_dict is not None + diffs_dict = {"differences": {}} nonmatching_versions = [] diff --git a/cfbs/masterfiles/download_all_versions.py b/cfbs/masterfiles/download_all_versions.py index ac9e6a6b..dde31952 100644 --- a/cfbs/masterfiles/download_all_versions.py +++ b/cfbs/masterfiles/download_all_versions.py @@ -15,7 +15,7 @@ def get_download_urls_enterprise(min_version=None): try: data = get_json(ENTERPRISE_RELEASES_URL) - except FetchError as e: + except FetchError: raise GenericExitError( "Downloading CFEngine release data failed - check your Wi-Fi / network settings." ) @@ -45,7 +45,7 @@ def get_download_urls_enterprise(min_version=None): release_url = release_data["URL"] try: subdata = get_json(release_url) - except FetchError as e: + except FetchError: raise GenericExitError( "Downloading CFEngine release data for version %s failed - check your Wi-Fi / network settings." % version diff --git a/cfbs/masterfiles/generate_release_information.py b/cfbs/masterfiles/generate_release_information.py index 3437ff1c..02b3403e 100644 --- a/cfbs/masterfiles/generate_release_information.py +++ b/cfbs/masterfiles/generate_release_information.py @@ -32,7 +32,8 @@ def generate_release_information(omit_download=False, check=False, min_version=N if check: print( - "Downloading releases of masterfiles from git (github.com) and generating additional release information for comparison..." + "Downloading releases of masterfiles from git (github.com) and generating " + + "additional release information for comparison..." ) generate_vcf_git_checkout(downloaded_versions) print("Candidate release information generated.") @@ -45,5 +46,6 @@ def generate_release_information(omit_download=False, check=False, min_version=N print("Release information successfully generated.") print("See the results in ./versions.json, ./checksums.json, and ./files.json") print( - "(Run again with --check-against-git to download and compare with files from git, and generate -git.json files)" + "(Run again with --check-against-git to download and compare with files " + + "from git, and generate -git.json files)" ) diff --git a/cfbs/masterfiles/generate_vcf_download.py b/cfbs/masterfiles/generate_vcf_download.py index 0e758369..54f4aa93 100644 --- a/cfbs/masterfiles/generate_vcf_download.py +++ b/cfbs/masterfiles/generate_vcf_download.py @@ -9,9 +9,11 @@ def generate_vcf_download(dir_path, downloaded_versions): - """`dir_path`: the path of the directory containing masterfiles versions subdirectories in the form `dir_path/x.y.z/tarball/` + """`dir_path`: the path of the directory containing masterfiles versions + subdirectories in the form `dir_path/x.y.z/tarball/` - The `tarball` folder should contain the `masterfiles` folder (older tarballs also have a `modules` folder alongside the `masterfiles` folder). + The `tarball` folder should contain the `masterfiles` folder (older + tarballs also have a `modules` folder alongside the `masterfiles` folder). """ versions_dict, checksums_dict, files_dict = initialize_vcf() diff --git a/cfbs/masterfiles/generate_vcf_git_checkout.py b/cfbs/masterfiles/generate_vcf_git_checkout.py index 6422bf8b..9a1ffa8e 100644 --- a/cfbs/masterfiles/generate_vcf_git_checkout.py +++ b/cfbs/masterfiles/generate_vcf_git_checkout.py @@ -61,7 +61,9 @@ def generate_vcf_git_checkout(checkout_tags): ) # build masterfiles from git as they are in the tarball packages - # for the files of this version to be reproducible, the `EXPLICIT_RELEASE` environment variable needs to be set to what it was when the downloadable files were built + # for the files of this version to be reproducible, the `EXPLICIT_RELEASE` + # environment variable needs to be set to what it was when the downloadable + # files were built if tag == "3.18.3": release_number = "2" else: diff --git a/cfbs/module.py b/cfbs/module.py index b25781aa..e5b0bb72 100644 --- a/cfbs/module.py +++ b/cfbs/module.py @@ -39,7 +39,7 @@ def __setattr__(self, name: str, value): def __getattr__(self, name: str): try: - return super().__getattr__(name) + return super().__getattribute__(name) except AttributeError as e: if name in Module.attributes(): return None diff --git a/cfbs/pretty.py b/cfbs/pretty.py index 0c415c19..4633ed5b 100644 --- a/cfbs/pretty.py +++ b/cfbs/pretty.py @@ -95,7 +95,7 @@ def _children_sort(child: OrderedDict, name, sorting_rules): } ``` - sorts the child objects in the given top-level (`name == None`) JSON object + sorts the child objects in the given top-level (`name is not None`) JSON object alphabetically by their name, then only inside the `"modules"` object all its (grand)child objects are sorted the same way and inside all (`".*"`) those (grand)child objects, their (grand-grand)child objects by the lengths @@ -183,7 +183,7 @@ def _item_index(iterable, item, extra_at_end=True): child_sorting_rules = rules[1] if child_sorting_rules is not None: for child_key, child_value in child.items(): - if type(child_value) == OrderedDict: + if type(child_value) is OrderedDict: _children_sort(child_value, child_key, child_sorting_rules) diff --git a/cfbs/updates.py b/cfbs/updates.py index 10f7d1fa..532be763 100644 --- a/cfbs/updates.py +++ b/cfbs/updates.py @@ -86,7 +86,7 @@ def _update_variable(input_def, input_data): if input_def["type"] == "list": def_subtype = input_def["subtype"] data_subtype = input_data["subtype"] - if type(def_subtype) != type(data_subtype): + if type(def_subtype) is not type(data_subtype): raise InputDataUpdateFailed( "Failed to update input data for module '%s': " % module_name + "Different subtypes in list ('%s' != '%s')." @@ -157,7 +157,7 @@ def update_module(old_module, new_module, module_updates, update): input_path = os.path.join(".", old_module["name"], "input.json") input_data = read_json(input_path) - if input_data == None: + if input_data is None: log.debug( "Skipping input update for module '%s': " % old_module["name"] + "No input found in '%s'" % input_path diff --git a/cfbs/utils.py b/cfbs/utils.py index 045eee2c..179506d9 100644 --- a/cfbs/utils.py +++ b/cfbs/utils.py @@ -7,8 +7,10 @@ import hashlib import urllib import urllib.request # needed on some platforms +import urllib.error from collections import OrderedDict from shutil import rmtree +from typing import Union from cfbs.pretty import pretty @@ -27,7 +29,7 @@ class GenericExitError(Exception): def _sh(cmd: str): # print(cmd) try: - r = subprocess.run( + subprocess.run( cmd, shell=True, check=True, @@ -100,7 +102,7 @@ def get_json(url: str) -> OrderedDict: raise FetchError("Failed to get JSON from '%s'" % url) from e -def get_or_read_json(path: str) -> OrderedDict: +def get_or_read_json(path: str) -> Union[OrderedDict, None]: if path.startswith("https://"): return get_json(path) return read_json(path) @@ -163,7 +165,7 @@ def save_file(path, data): f.write(data) -def read_json(path) -> OrderedDict: +def read_json(path) -> Union[OrderedDict, None]: try: with open(path, "r") as f: return json.loads(f.read(), object_pairs_hook=OrderedDict) @@ -232,8 +234,8 @@ def deduplicate_def_json(d): return d -def deduplicate_list(l): - return list(OrderedDict.fromkeys(l)) +def deduplicate_list(original): + return list(OrderedDict.fromkeys(original)) def dict_sorted_by_key(the_dict): @@ -272,21 +274,17 @@ def is_cfbs_repo() -> bool: def immediate_subdirectories(path): - l = [f.name for f in os.scandir(path) if f.is_dir()] + foldernames = [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 + return sorted(foldernames) def immediate_files(path): - l = [f.name for f in os.scandir(path) if not f.is_dir()] + filenames = [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 + return sorted(filenames) def path_append(dir, subdir): diff --git a/cfbs/validate.py b/cfbs/validate.py index 2f1855bc..8f3abd2f 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -205,7 +205,7 @@ def validate_build_step(name, module, i, operation, args, strict=False): "%s build step in '%s' is too long" % (operation, name) ) - if not operation in AVAILABLE_BUILD_STEPS: + if operation not in AVAILABLE_BUILD_STEPS: raise CFBSValidationError( name, 'Unknown operation "%s" in "steps", must be one of: %s (build step %s in module "%s")' @@ -249,7 +249,7 @@ def validate_build_step(name, module, i, operation, args, strict=False): if n > MAX_REPLACEMENTS or n == MAX_REPLACEMENTS and or_more: raise CFBSValidationError( "replace build step cannot replace something more than %s times" - % (MAX_REPLACEMENTS) + % (MAX_REPLACEMENTS,) ) if a in b: raise CFBSValidationError( @@ -294,7 +294,7 @@ def validate_build_step(name, module, i, operation, args, strict=False): assert type(filename) is str and filename != "" # replace_version requires the module to have a version field: - if not "version" in module: + if "version" not in module: raise CFBSValidationError( name, "Module '%s' missing \"version\" field for replace_version build step" @@ -325,7 +325,7 @@ def validate_alias(name, module, context): raise CFBSValidationError( name, '"alias" cannot be used with other attributes' ) - if type(module["alias"]) != str: + if type(module["alias"]) is not str: raise CFBSValidationError(name, '"alias" must be of type string') if not module["alias"]: raise CFBSValidationError(name, '"alias" must be non-empty') @@ -339,36 +339,36 @@ def validate_alias(name, module, context): def validate_name(name, module): assert "name" in module assert name == module["name"] - if type(module["name"]) != str: + if type(module["name"]) is not str: raise CFBSValidationError(name, '"name" must be of type string') if not module["name"]: raise CFBSValidationError(name, '"name" must be non-empty') def validate_description(name, module): assert "description" in module - if type(module["description"]) != str: + if type(module["description"]) is not str: raise CFBSValidationError(name, '"description" must be of type string') if not module["description"]: raise CFBSValidationError(name, '"description" must be non-empty') def validate_tags(name, module): assert "tags" in module - if type(module["tags"]) != list: + if type(module["tags"]) is not list: raise CFBSValidationError(name, '"tags" must be of type list') for tag in module["tags"]: - if type(tag) != str: + if type(tag) is not str: raise CFBSValidationError(name, '"tags" must be a list of strings') def validate_repo(name, module): assert "repo" in module - if type(module["repo"]) != str: + if type(module["repo"]) is not str: raise CFBSValidationError(name, '"repo" must be of type string') if not module["repo"]: raise CFBSValidationError(name, '"repo" must be non-empty') def validate_by(name, module): assert "by" in module - if type(module["by"]) != str: + if type(module["by"]) is not str: raise CFBSValidationError(name, '"by" must be of type string') if not module["by"]: raise CFBSValidationError(name, '"by" must be non-empty') @@ -382,12 +382,12 @@ def validate_dependencies(name, module, config, context): assert context == "index" search_in = ("index",) assert "dependencies" in module - if type(module["dependencies"]) != list: + if type(module["dependencies"]) is not list: raise CFBSValidationError( name, 'Value of attribute "dependencies" must be of type list' ) for dependency in module["dependencies"]: - if type(dependency) != str: + if type(dependency) is not str: raise CFBSValidationError( name, '"dependencies" must be a list of strings' ) @@ -404,23 +404,23 @@ def validate_dependencies(name, module, config, context): def validate_version(name, module): assert "version" in module - if type(module["version"]) != str: + if type(module["version"]) is not str: raise CFBSValidationError(name, '"version" must be of type string') regex = r"(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)(-([0-9]+))?" - if re.fullmatch(regex, module["version"]) == None: + if re.fullmatch(regex, module["version"]) is None: raise CFBSValidationError(name, '"version" must match regex %s' % regex) def validate_commit(name, module): assert "commit" in module commit = module["commit"] - if type(commit) != str: + if type(commit) is not str: raise CFBSValidationError(name, '"commit" must be of type string') if not is_a_commit_hash(commit): raise CFBSValidationError(name, '"commit" must be a commit reference') def validate_subdirectory(name, module): assert "subdirectory" in module - if type(module["subdirectory"]) != str: + if type(module["subdirectory"]) is not str: raise CFBSValidationError(name, '"subdirectory" must be of type string') if not module["subdirectory"]: raise CFBSValidationError(name, '"subdirectory" must be non-empty') @@ -437,12 +437,12 @@ def validate_subdirectory(name, module): def validate_steps(name, module): assert "steps" in module - if type(module["steps"]) != list: + if type(module["steps"]) is not list: raise CFBSValidationError(name, '"steps" must be of type list') if not module["steps"]: raise CFBSValidationError(name, '"steps" must be non-empty') for i, step in enumerate(module["steps"]): - if type(step) != str: + if type(step) is not str: raise CFBSValidationError(name, '"steps" must be a list of strings') if not step or step.strip() == "": raise CFBSValidationError( @@ -455,7 +455,7 @@ def validate_url_field(name, module, field): assert field in module url = module.get(field) if url and not url.startswith("https://"): - raise CFBSValidationError(name, '"%" must be an HTTPS URL' % field) + raise CFBSValidationError(name, '"%s" must be an HTTPS URL' % field) def validate_module_input(name, module): assert "input" in module @@ -516,7 +516,7 @@ def validate_module_input(name, module): ) if input_element["type"] == "list": - if not "while" in input_element: + if "while" not in input_element: raise CFBSValidationError( name, 'For a "list" input element, a "while" prompt is required' ) @@ -528,7 +528,7 @@ def validate_module_input(name, module): name, 'The "while" prompt in an input "list" element must be a non-empty / non-whitespace string', ) - if not "subtype" in input_element: + if "subtype" not in input_element: raise CFBSValidationError( name, 'For a "list" input element, a "subtype" is required' ) @@ -643,7 +643,7 @@ def validate_module_input(name, module): def _validate_config_for_build_field(config, empty_build_list_ok=False): """Validate that neccessary fields are in the config for the build/download commands to work""" - if not "build" in config: + if "build" not in config: raise GenericExitError( 'A "build" field is missing in ./cfbs.json' + " - The 'cfbs build' command loops through all modules in this list to find build steps to perform" diff --git a/tests/test_git.py b/tests/test_git.py index 4c416bdd..aeee1511 100644 --- a/tests/test_git.py +++ b/tests/test_git.py @@ -5,5 +5,5 @@ def test_ls_remote(): commit = ls_remote("https://github.com/cfengine/masterfiles.git", "master") print(commit) - assert commit != None + assert commit is not None assert is_a_commit_hash(commit) diff --git a/tests/test_module.py b/tests/test_module.py index 10bc7e42..1aa00496 100644 --- a/tests/test_module.py +++ b/tests/test_module.py @@ -10,7 +10,7 @@ def test_init(): module = Module("groups") assert module.name == "groups" - assert module.version == None + assert module.version is None def test_type_constraints(): diff --git a/tests/test_pretty.py b/tests/test_pretty.py index 712c1a77..f3c5bd04 100644 --- a/tests/test_pretty.py +++ b/tests/test_pretty.py @@ -187,7 +187,10 @@ def test_pretty_string(): assert pretty_string(test) == expected # Test wrapping based on siblings and length - test = '[{ "a": [1, 2], "b": [3, 4] }, { "d": [5, 6], "e": [7, 8, "This is a super duper long string which should cause wrapping due to its length"] }]' + test = ( + '[{ "a": [1, 2], "b": [3, 4] }, { "d": [5, 6], "e": ' + + '[7, 8, "This is a super duper long string which should cause wrapping due to its length"] }]' + ) expected = """[ { "a": [1, 2], @@ -209,14 +212,14 @@ def test_pretty_file(): mkdir("tests/tmp/", exist_ok=True) with open("tests/tmp/test_pretty_file.json", "w") as f: f.write(" {} \n") - assert pretty_file("tests/tmp/test_pretty_file.json") == True + assert pretty_file("tests/tmp/test_pretty_file.json") is True assert read_file("tests/tmp/test_pretty_file.json") == "{}\n" - assert pretty_file("tests/tmp/test_pretty_file.json") == False + assert pretty_file("tests/tmp/test_pretty_file.json") is False def test_pretty_check_string(): - assert pretty_check_string(' "Hello" ') == False - assert pretty_check_string('"Hello"') == True + assert pretty_check_string(' "Hello" ') is False + assert pretty_check_string('"Hello"') is True assert ( pretty_check_string( """{ @@ -224,9 +227,9 @@ def test_pretty_check_string(): "age": 27 }""" ) - == True + is True ) - assert pretty_check_string('{ "name": "lars", "age": 27 }') == False + assert pretty_check_string('{ "name": "lars", "age": 27 }') is False def test_pretty_sorting_simple_top_level(): diff --git a/tests/test_utils.py b/tests/test_utils.py index 042c436b..2c52b2ad 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -238,9 +238,9 @@ def test_deduplicate_def_json(): def test_deduplicate_list(): - l = [1, 2, 3, 3, 1, 4] + nums = [1, 2, 3, 3, 1, 4] - assert deduplicate_list(l) == [1, 2, 3, 4] + assert deduplicate_list(nums) == [1, 2, 3, 4] assert deduplicate_list([1, 1, 2, 3]) == [1, 2, 3] assert deduplicate_list([1, 2, 3, 3]) == [1, 2, 3] @@ -298,8 +298,8 @@ def test_are_paths_equal(): assert are_paths_equal(".", os.getcwd()) - assert are_paths_equal("a", "b") == False - assert are_paths_equal("a", "") == False + assert are_paths_equal("a", "b") is False + assert are_paths_equal("a", "") is False def test_string_sha256(): @@ -317,12 +317,12 @@ def test_file_sha256(): def test_is_a_commit_hash(): - assert is_a_commit_hash("304d123ac7ff50714a1eb57077acf159f923c941") == True + assert is_a_commit_hash("304d123ac7ff50714a1eb57077acf159f923c941") is True sha256_hash = "98142d6fa7e2e5f0942b0a215c1c4b976e7ae2ee5edb61cef974f1ba6756cbbc" - assert is_a_commit_hash(sha256_hash) == True + assert is_a_commit_hash(sha256_hash) is True # at least currently, commit cannot be a shortened hash - assert is_a_commit_hash("4738c43") == False - assert is_a_commit_hash("") == False + assert is_a_commit_hash("4738c43") is False + assert is_a_commit_hash("") is False def test_canonify(): diff --git a/tests/test_validate_index_alias.py b/tests/test_validate_index_alias.py index 0aa5a78e..86643dc8 100644 --- a/tests/test_validate_index_alias.py +++ b/tests/test_validate_index_alias.py @@ -1,6 +1,4 @@ import os -from collections import OrderedDict -from copy import deepcopy from cfbs.pretty import pretty from cfbs.validate import validate_config diff --git a/tests/test_validate_mock_input.py b/tests/test_validate_mock_input.py index 80524356..60cb0980 100644 --- a/tests/test_validate_mock_input.py +++ b/tests/test_validate_mock_input.py @@ -22,7 +22,9 @@ def warn_about_unknown_keys(self): "type": "index", "index": { "delete-files": { - "description": "Allows you to specify a list of files you want deleted on hosts in your infrastructure. When this module is deployed as part of your policy set, every time CFEngine runs, it will check if those files exist, and delete them if they do.", + "description": "Allows you to specify a list of files you want deleted on hosts in your infrastructure. " + + "When this module is deployed as part of your policy set, every time CFEngine runs, it will check if " + + "those files exist, and delete them if they do.", "tags": ["supported", "management"], "repo": "https://github.com/nickanderson/cfengine-delete-files", "by": "https://github.com/nickanderson",