fix(dfm): update traps and tests (#124)

* fix(dfm): update traps and tests * fix(dfm): initialize defaults and secure tests * fix(tests): secure helper quoting and extend install coverage * fix(utils): avoid double extension when resolving command * fix(tests): quote paths and add strict mode * fix(utils): escape function name in regex
2026-01-26 11:14:08 +00:00 · 2025-06-11 23:44:52 +03:00
parent 76076fdaa4
commit 1d0ea5ace4
6 changed files with 111 additions and 73 deletions
--- a/local/dfm/cmd/install.sh
+++ b/local/dfm/cmd/install.sh
@@ -1,5 +1,11 @@
 #!/usr/bin/env bash
 set -euo pipefail
+
+# Default paths can be overridden via environment variables
+: "${DOTFILES:=$HOME/.dotfiles}"
+: "${BREWFILE:=$DOTFILES/config/homebrew/Brewfile}"
+: "${TEMP_DIR:=$(mktemp -d)}"
+: "${DFM_MAX_RETRIES:=3}"
 # Installation functions for dfm, the dotfile manager
 #
 # @author Ismo Vuorinen <https://github.com/ivuorinen>
@@ -26,6 +32,9 @@ set -euo pipefail
 #
 # Example:
 #   all
+#
+# @description
+#   Parse command line options controlling installation steps.
 parse_options()
 {
  NO_AUTOMATION=0
@@ -56,8 +65,10 @@ parse_options()
  done
 }

-function all()
-{
+# @description
+#   Install all configured components by calling each individual
+#   installation routine unless skipped via options.
+function all() {
  parse_options "$@"

  lib::log "Installing all packages..."
@@ -91,8 +102,12 @@ function all()
 #
 # Example:
 #   fonts
-function fonts()
-{
+#
+# @description Install all configured fonts from helper script, prompting the user unless automation is disabled.
+function fonts() {
+
+  : "${SKIP_FONTS:=0}"
+  : "${NO_AUTOMATION:=0}"

  if [[ $SKIP_FONTS -eq 1 ]]; then
    lib::log "Skipping fonts installation"
@@ -126,8 +141,12 @@ function fonts()
 #
 # Example:
 #   brew
-function brew()
-{
+#
+# @description Install Homebrew and declared packages using the Brewfile.
+function brew() {
+
+  : "${SKIP_BREW:=0}"
+  : "${NO_AUTOMATION:=0}"


  if [[ $SKIP_BREW -eq 1 ]]; then
@@ -170,8 +189,12 @@ function brew()
 #
 # Example:
 #   cargo
-function cargo()
-{
+#
+# @description Install Rust tooling and cargo packages using helper scripts.
+function cargo() {
+
+  : "${SKIP_CARGO:=0}"
+  : "${NO_AUTOMATION:=0}"


  if [[ $SKIP_CARGO -eq 1 ]]; then
--- a/local/dfm/dfm
+++ b/local/dfm/dfm
@@ -3,26 +3,27 @@

 set -euo pipefail

-# define default variables
-DFM_SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+# allow overriding core directories
+DFM_SCRIPT_DIR="${DFM_SCRIPT_DIR:-$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)}"
 readonly DFM_SCRIPT_DIR
 export DFM_SCRIPT_DIR
-readonly DFM_CMD_DIR="${DFM_SCRIPT_DIR}/cmd"
+DFM_CMD_DIR="${DFM_CMD_DIR:-${DFM_SCRIPT_DIR}/cmd}"
+readonly DFM_CMD_DIR
 export DFM_CMD_DIR
-readonly DFM_LIB_DIR="${DFM_SCRIPT_DIR}/lib"
+DFM_LIB_DIR="${DFM_LIB_DIR:-${DFM_SCRIPT_DIR}/lib}"
+readonly DFM_LIB_DIR
 export DFM_LIB_DIR
-readonly DFM_DEFAULT_CONFIG_PATH="$HOME/.config"
+DFM_DEFAULT_CONFIG_PATH="${DFM_DEFAULT_CONFIG_PATH:-$HOME/.config}"
+readonly DFM_DEFAULT_CONFIG_PATH
 export DFM_DEFAULT_CONFIG_PATH
-readonly DFM_MAX_RETRIES=3
+DFM_MAX_RETRIES="${DFM_MAX_RETRIES:-3}"
+readonly DFM_MAX_RETRIES
 export DFM_MAX_RETRIES
-export DFM_DEFAULT_INSTALL_DIR="$HOME/.local"
-export DFM_DEFAULT_VERBOSE=0
-TEMP_DIR=$(mktemp -d)
+export DFM_DEFAULT_INSTALL_DIR="${DFM_DEFAULT_INSTALL_DIR:-$HOME/.local}"
+export DFM_DEFAULT_VERBOSE="${DFM_DEFAULT_VERBOSE:-0}"
+TEMP_DIR="${TEMP_DIR:-$(mktemp -d)}"
 export TEMP_DIR

-# Clean up temporary directory on exit
-trap 'rm -rf "$TEMP_DIR"' EXIT
-
 # Load the common and utility functions from the lib directory.
 [[ -f "${DFM_LIB_DIR}/common.sh" ]] || {
  echo "Error: Required file ${DFM_LIB_DIR}/common.sh not found"
--- a/local/dfm/lib/common.sh
+++ b/local/dfm/lib/common.sh
@@ -331,49 +331,22 @@ logger::error()
 #
 # Example:
 #   trap cleanup EXIT
-cleanup()
-{
-  local exit_code=$?
-  [ -d "$TEMP_DIR" ] && rm -rf "$TEMP_DIR"
-  exit $exit_code
+cleanup() {
+  local exit_code=${1:-$?}
+  if [[ -n ${TEMP_DIR:-} && -d $TEMP_DIR ]]; then
+    rm -rf "$TEMP_DIR"
+  fi
+  exit "$exit_code"
 }

 # Register the cleanup function to run on EXIT signal.
-#
-# This will ensure that temporary files and directories are removed
-# when the script exits or is interrupted by a signal (e.g. Ctrl+C).
-# The cleanup function is defined above.
+# This ensures temporary files and directories are removed
+# when the script exits or is interrupted.
 trap cleanup EXIT

 # Handle errors by logging an error message to the console.
-#
-# The function is registered with the `ERR` trap.
-# The line number where the error occurred is passed as an argument to the function.
-# The function is defined above.
-#
-# @description Handle an error
-# @param $1 Line number
-# Handles an error event by logging the line number and corresponding exit code.
-#
-# Globals:
-#   $? - The exit code of the last executed command.
-#
-# Arguments:
-#   $1 - The line number where the error occurred.
-#
-# Outputs:
-#   Logs an error message to STDERR via logger::error.
-#
-# Returns:
-#   None
+# The `ERR` trap passes the line number and command to lib::error::handle.
 #
 # Example:
-#   handle_error ${LINENO}
-handle_error()
-{
-  local exit_code=$?
-  local line_no=$1
-  logger::error "Failed at line ${line_no} with exit code ${exit_code}"
-}
-
-trap 'handle_error ${LINENO}' ERR
+#   lib::error::handle ${LINENO} "$BASH_COMMAND"
+trap 'lib::error::handle ${LINENO} "$BASH_COMMAND"' ERR
--- a/local/dfm/lib/utils.sh
+++ b/local/dfm/lib/utils.sh
@@ -673,19 +673,21 @@ main::get_command_functions()
 #   desc=$(main::get_function_description "install" "my_function")
 main::get_function_description()
 {
-  local cmd="$1"
+  local cmd_file="$1"
  local func="$2"
-  local cmd_file="${DFM_CMD_DIR}/${cmd}.sh"
-
-  if [[ ! -f "$cmd_file" && -f "$cmd" ]]; then
-    cmd_file="$cmd"
-  fi

  if [[ ! -f "$cmd_file" ]]; then
-    return 1
+    [[ -n ${DFM_CMD_DIR:-} ]] || return 1
+    cmd_file="${DFM_CMD_DIR}/${cmd_file}"
+    [[ "$cmd_file" == *.sh ]] || cmd_file="${cmd_file}.sh"
  fi

-  grep -B1 "^[[:space:]]*\(function[[:space:]]*\)\{0,1\}$func().*{" "$cmd_file" \
+  [[ -f "$cmd_file" ]] || return 1
+
+  local escaped_func
+  escaped_func=$(printf '%s' "$func" | sed 's/[][\\.^$*+?(){}|]/\\&/g')
+
+  grep -B5 -E "^[[:space:]]*(function[[:space:]]*)?${escaped_func}[[:space:]]*\\(\\)[[:space:]]*(\\{)?[[:space:]]*$" "$cmd_file" \
    | grep "@description" \
    | sed -E 's/^[[:space:]]*#[[:space:]]*@description[[:space:]]*//'
 }
--- a/tests/dfm/helpers.bash
+++ b/tests/dfm/helpers.bash
@@ -0,0 +1,17 @@
+run_with_dfm() {
+  local cmd="$*"
+  run env \
+    PROJECT_ROOT="$PROJECT_ROOT" \
+    DFM_CMD_DIR="$PROJECT_ROOT/local/dfm/cmd" \
+    DFM_LIB_DIR="$PROJECT_ROOT/local/dfm/lib" \
+    DOTFILES="${DOTFILES:-$PROJECT_ROOT}" \
+    NO_AUTOMATION="${NO_AUTOMATION:-1}" \
+    TEMP_DIR="$TEMP_DIR" \
+    bash -c 'set -e
+      cmd="$1"
+      source "$PROJECT_ROOT/local/dfm/lib/common.sh"
+      source "$PROJECT_ROOT/local/dfm/lib/utils.sh"
+      set +e
+      eval "$cmd"' bash "$cmd"
+}
+
--- a/tests/dfm/main.bats
+++ b/tests/dfm/main.bats
@@ -1,34 +1,56 @@
+load "$BATS_TEST_DIRNAME/helpers.bash"
+
 setup() {
+  set -euo pipefail
  PROJECT_ROOT="$BATS_TEST_DIRNAME/../.."
+  TEMP_DIR="$(mktemp -d)"
+  export TEMP_DIR
+}
+
+teardown() {
+  [[ -n "${TEMP_DIR:-}" ]] && rm -rf "$TEMP_DIR"
 }

@test "list_available_commands shows commands" {
-  run bash -c "export DFM_CMD_DIR=$PROJECT_ROOT/local/dfm/cmd; export DFM_LIB_DIR=$PROJECT_ROOT/local/dfm/lib; export TEMP_DIR=\$(mktemp -d); source $PROJECT_ROOT/local/dfm/lib/common.sh; source $PROJECT_ROOT/local/dfm/lib/utils.sh; set +e; main::list_available_commands"
+  run_with_dfm main::list_available_commands
  [ "$status" -eq 0 ]
  echo "$output" | grep -q "Available commands"
  echo "$output" | grep -q "install"
 }

@test "interactive confirm returns 0 on yes" {
-  run bash -c "source $PROJECT_ROOT/local/dfm/lib/utils.sh; set +e; utils::interactive::confirm \"Proceed?\" <<< \"y\"; echo \$?"
+  run_with_dfm 'utils::interactive::confirm "Proceed?" <<< "y"; echo $?'
  [ "$status" -eq 0 ]
  [ "${lines[-1]}" = "0" ]
 }

@test "interactive confirm returns 1 on no" {
-  run bash -c "source $PROJECT_ROOT/local/dfm/lib/utils.sh; set +e; utils::interactive::confirm \"Proceed?\" <<< \"n\"; echo \$?"
+  run_with_dfm 'utils::interactive::confirm "Proceed?" <<< "n"; echo $?'
  [ "$status" -eq 0 ]
  [ "${lines[-1]}" = "1" ]
 }

@test "execute_command runs function" {
-  run bash -c "export DFM_CMD_DIR=$PROJECT_ROOT/local/dfm/cmd; export DFM_LIB_DIR=$PROJECT_ROOT/local/dfm/lib; export TEMP_DIR=\$(mktemp -d); source $PROJECT_ROOT/local/dfm/lib/common.sh; source $PROJECT_ROOT/local/dfm/lib/utils.sh; set +e; main::execute_command install fonts"
+  run_with_dfm "main::execute_command install fonts"
  [ "$status" -eq 0 ]
  echo "$output" | grep -q "Installing fonts"
 }

@test "execute_command fails on missing function" {
-  run bash -c "export DFM_CMD_DIR=$PROJECT_ROOT/local/dfm/cmd; export DFM_LIB_DIR=$PROJECT_ROOT/local/dfm/lib; export TEMP_DIR=\$(mktemp -d); source $PROJECT_ROOT/local/dfm/lib/common.sh; source $PROJECT_ROOT/local/dfm/lib/utils.sh; set +e; main::execute_command install nofunc >/dev/null 2>&1; echo \$?"
-  [ "$status" -eq 0 ]
-  [ "${lines[-1]}" = "1" ]
+  run_with_dfm "main::execute_command install nofunc >/dev/null 2>&1"
+  [ "$status" -eq 1 ]
+}
+
+@test "install all respects skip options" {
+  run_with_dfm "main::execute_command install all --no-brew --no-cargo --no-automation"
+  [ "$status" -eq 0 ]
+  echo "$output" | grep -q "Installing fonts"
+  [[ "$output" != *"Installing Homebrew"* ]]
+  [[ "$output" != *"Rust and cargo packages"* ]]
+}
+
+@test "get_function_description returns description" {
+  run_with_dfm "main::get_function_description \"$PROJECT_ROOT/local/dfm/cmd/install.sh\" fonts"
+  [ "$status" -eq 0 ]
+  echo "$output" | grep -q "Install all configured fonts"
 }