Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: remove curl perl and coreutils as required dependencies #345

Merged
merged 7 commits into from
Oct 1, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ docker run --rm -it -v "$(pwd)":/project -w /project ubuntu:latest \
make test/alpine
# or
docker run --rm -it -v "$(pwd)":/project -w /project alpine:latest \
sh -c "apk add bash make shellcheck git curl perl && make test"
sh -c "apk add bash make shellcheck git && make test"
```

## Coding Guidelines
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ jobs:
run: |
docker run --rm -v "$(pwd)":/project alpine:latest /bin/sh -c " \
apk update && \
apk add --no-cache bash make git curl perl coreutils && \
apk add --no-cache bash make git && \
adduser -D builder && \
chown -R builder /project && \
su - builder -c 'cd /project; make test';"
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@
- Added defer expressions with `eval` when using standalone assertions
- Fixed simple output for non-successful states
- Remove deprecated assertions
- Some required dependencies now optional:
- perl
- coreutils
- Switch to testing the environment of capabilities rather than assuming various operating systems and Linux
distributions have programs installed.
- Upgrade and install script can now use `wget` if `curl` is not installed
- Tests can be also be timed by making use of `EPOCHREALTIME` on supported system.

## [0.16.0](https://github.com/TypedDevs/bashunit/compare/0.15.0...0.16.0) - 2024-09-15

Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ test/watch: $(TEST_SCRIPTS)

docker/alpine:
@docker run --rm -it -v "$(shell pwd)":/project -w /project alpine:latest \
sh -c "apk add bash make shellcheck git curl perl coreutils && bash"
sh -c "apk add bash make shellcheck git && bash"

env/example:
@echo "Copying variables without the values from .env into .env.example"
Expand Down
6 changes: 6 additions & 0 deletions bashunit
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ export BASHUNIT_ROOT_DIR

source "$BASHUNIT_ROOT_DIR/src/dev/debug.sh"
source "$BASHUNIT_ROOT_DIR/src/str.sh"
source "$BASHUNIT_ROOT_DIR/src/dependencies.sh"
source "$BASHUNIT_ROOT_DIR/src/io.sh"
source "$BASHUNIT_ROOT_DIR/src/math.sh"
source "$BASHUNIT_ROOT_DIR/src/default_env_config.sh"
source "$BASHUNIT_ROOT_DIR/src/env_configuration.sh"
source "$BASHUNIT_ROOT_DIR/src/check_os.sh"
Expand All @@ -30,6 +33,9 @@ _ASSERT_FN=""
_FILTER=""
_ARGS=()

check_os::init
clock::init

while [[ $# -gt 0 ]]; do
argument="$1"
case $argument in
Expand Down
8 changes: 7 additions & 1 deletion install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,13 @@ function install() {
echo "> Downloading the latest version: '$TAG'"
fi

curl -L -O -J "https://github.com/TypedDevs/bashunit/releases/download/$TAG/bashunit" 2>/dev/null
if command -v curl > /dev/null 2>&1; then
curl -L -O -J "https://github.com/TypedDevs/bashunit/releases/download/$TAG/bashunit" 2>/dev/null
elif command -v wget > /dev/null 2>&1; then
wget "https://github.com/TypedDevs/bashunit/releases/download/$TAG/bashunit" 2>/dev/null
else
echo "Cannot download bashunit: curl or wget not found."
fi
chmod u+x "bashunit"
}

Expand Down
67 changes: 55 additions & 12 deletions src/check_os.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,63 @@
_OS="Unknown"
_DISTRO="Unknown"

if [[ "$(uname)" == "Linux" ]]; then
_OS="Linux"
if command -v apt > /dev/null; then
_DISTRO="Ubuntu"
elif command -v apk > /dev/null; then
_DISTRO="Alpine"
function check_os::init() {
if check_os::is_linux; then
_OS="Linux"
if check_os::is_ubuntu; then
_DISTRO="Ubuntu"
elif check_os::is_alpine; then
_DISTRO="Alpine"
else
_DISTRO="Other"
fi
elif check_os::is_macos; then
_OS="OSX"
elif check_os::is_windows; then
_OS="Windows"
else
_DISTRO="Other"
_OS="Unknown"
_DISTRO="Unknown"
fi
elif [[ "$(uname)" == "Darwin" ]]; then
_OS="OSX"
elif [[ "$(uname)" == *"MINGW"* ]]; then
_OS="Windows"
fi
}

function check_os::is_ubuntu() {
command -v apt > /dev/null
}

function check_os::is_alpine() {
command -v apk > /dev/null
}

function check_os::is_linux() {
[[ "$(uname)" == "Linux" ]]
}

function check_os::is_macos() {
[[ "$(uname)" == "Darwin" ]]
}

function check_os::is_windows() {
[[ "$(uname)" == *"MINGW"* ]]
}

function check_os::is_busybox() {

case "$_DISTRO" in

"Alpine")
return 0
;;
*)
return 1
Comment on lines +52 to +55
Copy link
Member

@Chemaclass Chemaclass Oct 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might have notice we are using everywhere in the project checking true|false for conditional booleans. However, they are raw strings are bools are not natively supported by bash. On the contrary, native conditions in bash are 0:true,>=1:false which is a bit odd if you come from other languages. In the end this is purely a detail implementation from the bashunit insides, so it should not matter to the external user.

I am saying this because I am wondering if there is a performance gain/benefit on using 0/1 for booleans intead of true/false in which case I would be in favor of refactoring the bools inside the project to use them. Otherwise, I would suggest keep using true/false. What do you think, @skinner-m-c ?

I am ok either way, I want to learn from your experience, in case you know what's better :)

Anyway, THIS comment should NOT be applied on this PR but in a follow up iteration.

Copy link
Contributor Author

@skinner-m-c skinner-m-c Oct 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that it is confusing that 0 being true and 1 being false is confusing. However it comes from Unix standard of program exit codes -- that use 0 meaning "normal exit and not errors" and anything else is an exit where the program executed with some problems.

true and false are programs, typically installed at /bin/true and /bin/false. They are provided by coreutils or busybox. Both of these programs are very simple and are unlikely to cause performance issues. There may be more of a performance hit starting these programs than their execution. See true.c in coreutils and busybox for what they actually do. So it is probably faster to use 0 and 1 for comparisons. When true and false are returned it is the output of these programs.

However, BASH also has built-in functions and that is what is actually being used here and so /bin/true and /bin/false is not actually being run. The built-in true and false is probably marginally slower than integers (see help in BASH to see the list of all built-in functions). I don't have any metric to support that idea, but it is probably too small to warrant choosing 0 over false for example. false and true also are more readable.

I agree that true and false should probably be used.

Copy link
Member

@Chemaclass Chemaclass Oct 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In such a case, do you mind creating a PR refactoring/changing this to use true/false then? Then we can create an ADR to write down this as a project convention, so we can keep consistency over the project when dealing with booleans.

Extra thought: it's not the same true and "true", right? one is a program and the other one is a string, I assume, but I believe they are compatible and working intermixed? anyway, that's why the ADR is important to keep consistency.

Copy link
Member

@Chemaclass Chemaclass Oct 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been thinking about this topic, and while I think it's a bit odd thinking this way if you are not familiar with sh/bash, and after more than a year working on the project and a lot of researching, I ended up believing that 0 (as true) and 1 (as false) is a good practice in bash itself, specially when defining conditions.


While you can do this in bash

function check_os::is_busybox() {
  case "$_DISTRO" in
    "Alpine")
        return 0
        ;;
    *)
      return 1
      ;;
  esac
}

but you cannot do this:

Screenshot 2024-10-03 at 16 01 52

TL;DR I would be in favor of using 0 (as true) and 1 (as false) when dealing with conditionals in the project. I will create an ADR for this and refactor the project to follow this convention.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow up: #346

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also gave me the idea to add assert_true and assert_false #350

Copy link
Contributor Author

@skinner-m-c skinner-m-c Oct 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glad it is inspiring some more improvements.

true and false are build-in functions in BASH. Functions in BASH cannot be returned (though you can return a string of a function name and then invoke it with eval). However, the return value of the last function is returned if return has no value. So it would be a two-liner or a one liner with a subshell. Subshells should be avoided if possible because it is slower.

function abc() {
  true
  return
function abc() (
  return $(true)
}

Copy link
Member

@Chemaclass Chemaclass Oct 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mind reviewing the last pr where I introduced assert_true and false? I would like your feedback:)

;;
esac
}

check_os::init

export _OS
export _DISTRO
export -f check_os::is_alpine
export -f check_os::is_busybox
export -f check_os::is_ubuntu
49 changes: 42 additions & 7 deletions src/clock.sh
Original file line number Diff line number Diff line change
@@ -1,22 +1,57 @@
#!/bin/bash

function clock::now() {
if perl -MTime::HiRes -e "" > /dev/null 2>&1; then
perl -MTime::HiRes -e 'printf("%.0f\n", Time::HiRes::time() * 1000)'
elif [[ "${_OS:-}" != "OSX" ]]; then
if dependencies::has_perl && perl -MTime::HiRes -e "" > /dev/null 2>&1; then
if perl -MTime::HiRes -e 'printf("%.0f\n",Time::HiRes::time()*1000000000)'; then
return 0
fi
fi

if ! check_os::is_macos && ! check_os::is_alpine; then
date +%s%N
else
echo ""
return 0
fi

local shell_time has_shell_time
shell_time="$(clock::shell_time)"
has_shell_time="$?"
if [[ "$has_shell_time" -eq 0 ]]; then
local seconds microseconds
seconds=$(echo "$shell_time" | cut -f 1 -d '.')
microseconds=$(echo "$shell_time" | cut -f 2 -d '.')

math::calculate "($seconds * 1000000000) + ($microseconds * 1000)"
return 0
fi

echo ""
return 1
}

function clock::shell_time() {
# Get time directly from the shell rather than a program.
[[ -n ${EPOCHREALTIME+x} && -n "$EPOCHREALTIME" ]] && LC_ALL=C echo "$EPOCHREALTIME"
}

_START_TIME=$(clock::now)

function clock::total_runtime_in_milliseconds() {
end_time=$(clock::now)
if [[ -n $end_time ]]; then
echo $(( end_time - _START_TIME ))
math::calculate "($end_time-$_START_TIME)/1000000"
else
echo ""
fi
}

function clock::total_runtime_in_nanoseconds() {
end_time=$(clock::now)
if [[ -n $end_time ]]; then
math::calculate "($end_time-$_START_TIME)"
else
echo ""
fi
}

function clock::init() {
_START_TIME=$(clock::now)
}
2 changes: 1 addition & 1 deletion src/console_results.sh
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ function console_results::print_failed_snapshot_test() {
line="$(printf "${_COLOR_FAILED}βœ— Failed${_COLOR_DEFAULT}: %s
${_COLOR_FAINT}Expected to match the snapshot${_COLOR_DEFAULT}\n" "$function_name")"

if command -v git > /dev/null; then
if dependencies::has_git; then
local actual_file="${snapshot_file}.tmp"
echo "$actual" > "$actual_file"

Expand Down
30 changes: 30 additions & 0 deletions src/dependencies.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#!/bin/bash
set -euo pipefail

function dependencies::has_perl() {
command -v perl >/dev/null 2>&1
}

function dependencies::has_adjtimex() {
command -v adjtimex >/dev/null 2>&1
}

function dependencies::has_bc() {
command -v bc >/dev/null 2>&1
}

function dependencies::has_awk() {
command -v awk >/dev/null 2>&1
}

function dependencies::has_git() {
command -v git >/dev/null 2>&1
}

function dependencies::has_curl() {
command -v curl >/dev/null 2>&1
}

function dependencies::has_wget() {
command -v wget >/dev/null 2>&1
}
13 changes: 13 additions & 0 deletions src/io.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#!/bin/bash

function io::download_to() {
local url="$1"
local output="$2"
if dependencies::has_curl; then
curl -L -J -o "$output" "$url" 2>/dev/null
elif dependencies::has_wget; then
wget -q -O "$output" "$url" 2>/dev/null
else
return 1
fi
}
12 changes: 12 additions & 0 deletions src/math.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#!/bin/bash

if dependencies::has_bc; then
# bc is better than awk because bc has no integer limits.
function math::calculate() {
echo "$*" | bc
}
elif dependencies::has_awk; then
function math::calculate() {
awk "BEGIN { print ""$*"" }"
}
fi
8 changes: 6 additions & 2 deletions src/runner.sh
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ function runner::run_test() {
echo "$test_execution_result" |\
tail -n 1 |\
sed -E -e 's/.*##TEST_OUTPUT=(.*)##.*/\1/g' |\
base64 --decode
base64 -d
)

if [[ -n "$subshell_output" ]]; then
Expand Down Expand Up @@ -213,7 +213,11 @@ function runner::run_test() {

local end_time
end_time=$(clock::now)
local duration=$((end_time - start_time))
local duration
local duration_ns
duration_ns=$(math::calculate "($end_time - $start_time) ")
duration=$(math::calculate "$duration_ns / 1000000")
#

if [[ -n $runtime_error ]]; then
state::add_tests_failed
Expand Down
6 changes: 5 additions & 1 deletion src/upgrade.sh
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@ function upgrade::upgrade() {

echo "> Upgrading bashunit to latest version"
cd "$script_path" || exit
curl -L -J -o bashunit "https://github.com/TypedDevs/bashunit/releases/download/$latest_tag/bashunit" 2>/dev/null

if ! io::download_to "https://github.com/TypedDevs/bashunit/releases/download/$latest_tag/bashunit" "bashunit"; then
echo "Failed to download bashunit"
fi

chmod u+x "bashunit"

echo "> bashunit upgraded successfully to latest version $latest_tag"
Expand Down
64 changes: 64 additions & 0 deletions tests/globals.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,67 @@ function current_dir() {
function current_filename() {
basename "${BASH_SOURCE[1]}"
}


function mock_non_existing_fn() {
return 127;
}

function mock_false() {
return 1;
}

function mock_true() {
return 0;
}

function mock_unknown_linux_os() {
mock check_os::is_linux mock_true

mock check_os::is_ubuntu mock_false
mock check_os::is_alpine mock_false
mock check_os::is_busybox mock_false
mock check_os::is_macos mock_false
mock check_os::is_windows mock_false
}


function mock_ubuntu_os() {
mock check_os::is_linux mock_true
mock check_os::is_ubuntu mock_true

mock check_os::is_alpine mock_false
mock check_os::is_busybox mock_false
mock check_os::is_macos mock_false
mock check_os::is_windows mock_false
}

function mock_alpine_os() {
mock check_os::is_linux mock_true
mock check_os::is_alpine mock_true
mock check_os::is_busybox mock_true

mock check_os::is_ubuntu mock_false
mock check_os::is_macos mock_false
mock check_os::is_windows mock_false
}

function mock_macos() {
mock check_os::is_macos mock_true

mock check_os::is_linux mock_false
mock check_os::is_alpine mock_false
mock check_os::is_ubuntu mock_false
mock check_os::is_busybox mock_false
mock check_os::is_windows mock_false
}

function mock_windows_os() {
mock check_os::is_windows mock_true

mock check_os::is_linux mock_false
mock check_os::is_alpine mock_false
mock check_os::is_ubuntu mock_false
mock check_os::is_busybox mock_false
mock check_os::is_is_macos mock_false
}
Loading
Loading