chore: add some exception handling minor cleanup checks

Signed-off-by: Henry Schreiner <[email protected]>

pyproject.toml

@@ -159,6 +159,8 @@ select = [
   "UP",   # pyupgrade
   "W",    # pycodestyle
   "YTT",  # flake8-2020
+  "TRY",  # tryceratops
+  "EM",   # flake8-errmsg
 ]
 src = ["src"]
 

src/build/__init__.py

@@ -68,11 +68,13 @@ def _find_typo(dictionary: Mapping[str, str], expected: str) -> None:
 
 def _validate_source_directory(source_dir: PathType) -> None:
     if not os.path.isdir(source_dir):
-        raise BuildException(f'Source {source_dir} is not a directory')
+        msg = f'Source {source_dir} is not a directory'
+        raise BuildException(msg)
     pyproject_toml = os.path.join(source_dir, 'pyproject.toml')
     setup_py = os.path.join(source_dir, 'setup.py')
     if not os.path.exists(pyproject_toml) and not os.path.exists(setup_py):
-        raise BuildException(f'Source {source_dir} does not appear to be a Python project: no pyproject.toml or setup.py')
+        msg = f'Source {source_dir} does not appear to be a Python project: no pyproject.toml or setup.py'
+        raise BuildException(msg)
 
 
 def _read_pyproject_toml(path: PathType) -> Mapping[str, Any]:
@@ -82,9 +84,11 @@ def _read_pyproject_toml(path: PathType) -> Mapping[str, Any]:
     except FileNotFoundError:
         return {}
     except PermissionError as e:
-        raise BuildException(f"{e.strerror}: '{e.filename}' ")  # noqa: B904 # use raise from
+        msg = f"{e.strerror}: '{e.filename}' "
+        raise BuildException(msg) from None
     except tomllib.TOMLDecodeError as e:
-        raise BuildException(f'Failed to parse {path}: {e} ')  # noqa: B904 # use raise from
+        msg = f'Failed to parse {path}: {e} '
+        raise BuildException(msg) from None
 
 
 def _parse_build_system_table(pyproject_toml: Mapping[str, Any]) -> Mapping[str, Any]:
@@ -99,29 +103,34 @@ def _parse_build_system_table(pyproject_toml: Mapping[str, Any]) -> Mapping[str,
     # If [build-system] is present, it must have a ``requires`` field (per PEP 518)
     if 'requires' not in build_system_table:
         _find_typo(build_system_table, 'requires')
-        raise BuildSystemTableValidationError('`requires` is a required property')
+        msg = '`requires` is a required property'
+        raise BuildSystemTableValidationError(msg)
     elif not isinstance(build_system_table['requires'], list) or not all(
         isinstance(i, str) for i in build_system_table['requires']
     ):
-        raise BuildSystemTableValidationError('`requires` must be an array of strings')
+        msg = '`requires` must be an array of strings'
+        raise BuildSystemTableValidationError(msg)
 
     if 'build-backend' not in build_system_table:
         _find_typo(build_system_table, 'build-backend')
         # If ``build-backend`` is missing, inject the legacy setuptools backend
         # but leave ``requires`` intact to emulate pip
         build_system_table['build-backend'] = _DEFAULT_BACKEND['build-backend']
     elif not isinstance(build_system_table['build-backend'], str):
-        raise BuildSystemTableValidationError('`build-backend` must be a string')
+        msg = '`build-backend` must be a string'
+        raise BuildSystemTableValidationError(msg)
 
     if 'backend-path' in build_system_table and (
         not isinstance(build_system_table['backend-path'], list)
         or not all(isinstance(i, str) for i in build_system_table['backend-path'])
     ):
-        raise BuildSystemTableValidationError('`backend-path` must be an array of strings')
+        msg = '`backend-path` must be an array of strings'
+        raise BuildSystemTableValidationError(msg)
 
     unknown_props = build_system_table.keys() - {'requires', 'build-backend', 'backend-path'}
     if unknown_props:
-        raise BuildSystemTableValidationError(f'Unknown properties: {", ".join(unknown_props)}')
+        msg = f'Unknown properties: {", ".join(unknown_props)}'
+        raise BuildSystemTableValidationError(msg)
 
     return build_system_table
 
@@ -307,7 +316,8 @@ def metadata_path(self, output_directory: PathType) -> str:
         wheel = self.build('wheel', output_directory)
         match = parse_wheel_filename(os.path.basename(wheel))
         if not match:
-            raise ValueError('Invalid wheel')
+            msg = 'Invalid wheel'
+            raise ValueError(msg)
         distinfo = f"{match['distribution']}-{match['version']}.dist-info"
         member_prefix = f'{distinfo}/'
         with zipfile.ZipFile(wheel) as w:
@@ -326,7 +336,8 @@ def _call_backend(
 
         if os.path.exists(outdir):
             if not os.path.isdir(outdir):
-                raise BuildException(f"Build path '{outdir}' exists and is not a directory")
+                msg = f"Build path '{outdir}' exists and is not a directory"
+                raise BuildException(msg)
         else:
             os.makedirs(outdir)
 
@@ -340,17 +351,15 @@ def _handle_backend(self, hook: str) -> Iterator[None]:
         try:
             yield
         except pyproject_hooks.BackendUnavailable as exception:
-            raise BuildBackendException(  # noqa: B904 # use raise from
+            raise BuildBackendException(
                 exception,
                 f"Backend '{self._backend}' is not available.",
                 sys.exc_info(),
-            )
+            ) from None
         except subprocess.CalledProcessError as exception:
-            raise BuildBackendException(  # noqa: B904 # use raise from
-                exception, f'Backend subprocess exited when trying to invoke {hook}'
-            )
+            raise BuildBackendException(exception, f'Backend subprocess exited when trying to invoke {hook}') from None
         except Exception as exception:
-            raise BuildBackendException(exception, exc_info=sys.exc_info())  # noqa: B904 # use raise from
+            raise BuildBackendException(exception, exc_info=sys.exc_info()) from None
 
     @staticmethod
     def log(message: str) -> None:

src/build/__main__.py

@@ -176,7 +176,8 @@ def _handle_build_error() -> Iterator[None]:
 
 def _natural_language_list(elements: Sequence[str]) -> str:
     if len(elements) == 0:
-        raise IndexError('no elements')
+        msg = 'no elements'
+        raise IndexError(msg)
     elif len(elements) == 1:
         return elements[0]
     else:
@@ -232,7 +233,8 @@ def build_package_via_sdist(
     from ._util import TarFile
 
     if 'sdist' in distributions:
-        raise ValueError('Only binary distributions are allowed but sdist was specified')
+        msg = 'Only binary distributions are allowed but sdist was specified'
+        raise ValueError(msg)
 
     sdist = _build(isolation, srcdir, outdir, 'sdist', config_settings, skip_dependency_check)
 

src/build/env.py

@@ -64,7 +64,7 @@ def _subprocess(cmd: list[str]) -> None:
         subprocess.run(cmd, check=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
     except subprocess.CalledProcessError as e:
         print(e.output.decode(), end='', file=sys.stderr)
-        raise e
+        raise
 
 
 class DefaultIsolatedEnv(IsolatedEnv):
@@ -87,11 +87,12 @@ def __enter__(self) -> DefaultIsolatedEnv:
                 # Ref: https://bugs.python.org/issue46171
                 self._path = os.path.realpath(tempfile.mkdtemp(prefix='build-env-'))
                 self._python_executable, self._scripts_dir = _create_isolated_env_venv(self._path)
-            return self
         except Exception:  # cleanup folder if creation fails
             self.__exit__(*sys.exc_info())
             raise
 
+        return self
+
     def __exit__(self, *args: object) -> None:
         if os.path.exists(self._path):  # in case the user already deleted skip remove
             shutil.rmtree(self._path)
@@ -188,9 +189,9 @@ def _fs_supports_symlink() -> bool:
         try:
             os.symlink(tmp_file.name, dest)
             os.unlink(dest)
-            return True
         except (OSError, NotImplementedError, AttributeError):
             return False
+        return True
 
 
 def _create_isolated_env_venv(path: str) -> tuple[str, str]:
@@ -277,7 +278,8 @@ def _find_executable_and_scripts(path: str) -> tuple[str, str, str]:
         paths = sysconfig.get_paths(vars=config_vars)
     executable = os.path.join(paths['scripts'], 'python.exe' if sys.platform.startswith('win') else 'python')
     if not os.path.exists(executable):
-        raise RuntimeError(f'Virtual environment creation failed, executable {executable} missing')
+        msg = f'Virtual environment creation failed, executable {executable} missing'
+        raise RuntimeError(msg)
 
     return executable, paths['scripts'], paths['purelib']
 

tests/conftest.py

@@ -38,7 +38,8 @@ def pytest_collection_modifyitems(config, items):
     skip_other = pytest.mark.skip(reason='only integration tests are run (got --only-integration flag)')
 
     if config.getoption('--run-integration') and config.getoption('--only-integration'):  # pragma: no cover
-        raise pytest.UsageError("--run-integration and --only-integration can't be used together, choose one")
+        msg = "--run-integration and --only-integration can't be used together, choose one"
+        raise pytest.UsageError(msg)
 
     if len(items) == 1:  # do not require flags if called directly
         return

tests/packages/test-cant-build-via-sdist/backend_bad_sdist.py

@@ -16,7 +16,8 @@ def build_sdist(sdist_directory, config_settings=None):
 
 def build_wheel(wheel_directory, config_settings=None, metadata_directory=None):
     if not os.path.isfile('some-file-that-is-needed-for-build.txt'):
-        raise FileNotFoundError('some-file-that-is-needed-for-build.txt is missing!')
+        msg = 'some-file-that-is-needed-for-build.txt is missing!'
+        raise FileNotFoundError(msg)
     # pragma: no cover
     file = 'test_cant_build_via_sdist-1.0.0-py2.py3-none-any.whl'
     zipfile.ZipFile(os.path.join(wheel_directory, file), 'w').close()

tests/test_main.py

@@ -98,8 +98,9 @@ def test_parse_args(mocker, cli_args, build_args, hook):
         build.__main__.build_package.assert_called_with(*build_args)
     elif hook == 'build_package_via_sdist':
         build.__main__.build_package_via_sdist.assert_called_with(*build_args)
-    else:
-        raise ValueError(f'Unknown hook {hook}')  # pragma: no cover
+    else:  # pragma: no cover
+        msg = f'Unknown hook {hook}'
+        raise ValueError(msg)
 
 
 def test_prog():

tests/test_main_helpers.py

@@ -0,0 +1,11 @@
+import pytest
+
+from build.__main__ import _natural_language_list
+
+
+def test_natural_language_list():
+    assert _natural_language_list(['one']) == 'one'
+    assert _natural_language_list(['one', 'two']) == 'one and two'
+    assert _natural_language_list(['one', 'two', 'three']) == 'one, two and three'
+    with pytest.raises(IndexError, match='no elements'):
+        _natural_language_list([])

tests/test_projectbuilder.py

@@ -487,7 +487,8 @@ def test_no_outdir_multiple(mocker, tmp_dir, package_test_flit):
 
 def test_runner_user_specified(tmp_dir, package_test_flit):
     def dummy_runner(cmd, cwd=None, extra_environ=None):
-        raise RuntimeError('Runner was called')
+        msg = 'Runner was called'
+        raise RuntimeError(msg)
 
     builder = build.ProjectBuilder(package_test_flit, runner=dummy_runner)
     with pytest.raises(build.BuildBackendException, match='Runner was called'):