From 1d0ea5ace442aa959bd0785d7e470f77e64a5520 Mon Sep 17 00:00:00 2001 From: Ismo Vuorinen Date: Wed, 11 Jun 2025 23:44:52 +0300 Subject: [PATCH] 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 --- local/dfm/cmd/install.sh | 39 +++++++++++++++++++++++++------- local/dfm/dfm | 25 ++++++++++---------- local/dfm/lib/common.sh | 49 +++++++++------------------------------- local/dfm/lib/utils.sh | 18 ++++++++------- tests/dfm/helpers.bash | 17 ++++++++++++++ tests/dfm/main.bats | 36 +++++++++++++++++++++++------ 6 files changed, 111 insertions(+), 73 deletions(-) create mode 100644 tests/dfm/helpers.bash diff --git a/local/dfm/cmd/install.sh b/local/dfm/cmd/install.sh index e1ff466..eae2119 100644 --- 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 @@ -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 diff --git a/local/dfm/dfm b/local/dfm/dfm index 2298a54..3f6099d 100755 --- 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" diff --git a/local/dfm/lib/common.sh b/local/dfm/lib/common.sh index cfcb89a..0589ed6 100644 --- 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 diff --git a/local/dfm/lib/utils.sh b/local/dfm/lib/utils.sh index 7eab040..820ae01 100644 --- 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:]]*//' } diff --git a/tests/dfm/helpers.bash b/tests/dfm/helpers.bash new file mode 100644 index 0000000..aa1bc7c --- /dev/null +++ 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" +} + diff --git a/tests/dfm/main.bats b/tests/dfm/main.bats index d3647b9..f185d94 100644 --- 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" }