Skip to content

🧹 Refactor duplicated bash test helper variables and check function #50

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.

Sign up for GitHub

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

Open
savvides wants to merge 1 commit into
base: main
Choose a base branch
from
Open

🧹 Refactor duplicated bash test helper variables and check function #50

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 1 addition & 14 deletions test/integration-test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,12 @@
# idstack integration tests — behavioral tests for bin scripts
set -e

PASS=0
FAIL=0
TOTAL=0
. "$(dirname "$0")/test_helper.sh"

IDSTACK_DIR="$(cd "$(dirname "$0")/.." && pwd)"
TEST_DIR=$(mktemp -d)
trap "rm -rf $TEST_DIR" EXIT

check() {
TOTAL=$((TOTAL + 1))
if eval "$2" 2>/dev/null; then
echo " PASS: $1"
PASS=$((PASS + 1))
else
echo " FAIL: $1"
FAIL=$((FAIL + 1))
fi
}

echo "idstack integration tests"
echo " test dir: $TEST_DIR"
echo ""
Expand Down
15 changes: 1 addition & 14 deletions test/smoke-test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,12 @@
# idstack smoke test — verifies installation is correct
set -e

PASS=0
FAIL=0
TOTAL=0
. "$(dirname "$0")/test_helper.sh"

# Verify the repo this script lives in (test/smoke-test.sh -> repo root is "..").
# Override with $1 to point at a different checkout (CI fixtures, etc.).
IDSTACK_DIR="${1:-$(cd "$(dirname "$0")/.." && pwd -P)}"

check() {
TOTAL=$((TOTAL + 1))
if eval "$2" 2>/dev/null; then
echo " PASS: $1"
PASS=$((PASS + 1))
else
echo " FAIL: $1"
FAIL=$((FAIL + 1))
fi
}

echo "idstack smoke test"
echo " idstack dir: $IDSTACK_DIR"
echo ""
Expand Down
45 changes: 16 additions & 29 deletions test/test-manifest-merge.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,11 @@

set -e

PASS=0
FAIL=0
TOTAL=0
. "$(dirname "$0")/test_helper.sh"

REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)"
MERGE="$REPO_ROOT/bin/idstack-manifest-merge"

assert() {
TOTAL=$((TOTAL + 1))
if eval "$2" >/dev/null 2>&1; then
PASS=$((PASS + 1))
echo " PASS: $1"
else
FAIL=$((FAIL + 1))
echo " FAIL: $1"
fi
}

# Skip the suite if python3 is missing (the tool requires python3).
if ! command -v python3 >/dev/null 2>&1; then
echo "test-manifest-merge: python3 not available, skipping"
Expand Down Expand Up @@ -58,33 +45,33 @@ echo ""
seed_manifest
echo '{"confidence_score": 75}' > "$WORK/payload.json"
"$MERGE" --section red_team_audit --payload "$WORK/payload.json" --manifest "$WORK/project.json" --quiet
assert "section replaced" \
check "section replaced" \
"python3 -c 'import json; d=json.load(open(\"$WORK/project.json\")); assert d[\"red_team_audit\"]=={\"confidence_score\": 75}'"

# --- Test 2: foreign sections preserved ---
assert "context preserved" "python3 -c 'import json; d=json.load(open(\"$WORK/project.json\")); assert d[\"context\"]=={\"modality\": \"online\"}'"
assert "needs_analysis preserved" "python3 -c 'import json; d=json.load(open(\"$WORK/project.json\")); assert d[\"needs_analysis\"]=={\"existing\": \"data\"}'"
check "context preserved" "python3 -c 'import json; d=json.load(open(\"$WORK/project.json\")); assert d[\"context\"]=={\"modality\": \"online\"}'"
check "needs_analysis preserved" "python3 -c 'import json; d=json.load(open(\"$WORK/project.json\")); assert d[\"needs_analysis\"]=={\"existing\": \"data\"}'"

# --- Test 3: top-level fields preserved (project_name, created) ---
assert "project_name preserved" "python3 -c 'import json; d=json.load(open(\"$WORK/project.json\")); assert d[\"project_name\"]==\"test-project\"'"
assert "created preserved" "python3 -c 'import json; d=json.load(open(\"$WORK/project.json\")); assert d[\"created\"]==\"2026-01-01T00:00:00Z\"'"
assert "version preserved" "python3 -c 'import json; d=json.load(open(\"$WORK/project.json\")); assert d[\"version\"]==\"1.4\"'"
check "project_name preserved" "python3 -c 'import json; d=json.load(open(\"$WORK/project.json\")); assert d[\"project_name\"]==\"test-project\"'"
check "created preserved" "python3 -c 'import json; d=json.load(open(\"$WORK/project.json\")); assert d[\"created\"]==\"2026-01-01T00:00:00Z\"'"
check "version preserved" "python3 -c 'import json; d=json.load(open(\"$WORK/project.json\")); assert d[\"version\"]==\"1.4\"'"

# --- Test 4: updated timestamp bumped (not equal to seeded value) ---
assert "updated timestamp bumped" "python3 -c 'import json; d=json.load(open(\"$WORK/project.json\")); assert d[\"updated\"]!=\"2026-01-01T00:00:00Z\"'"
check "updated timestamp bumped" "python3 -c 'import json; d=json.load(open(\"$WORK/project.json\")); assert d[\"updated\"]!=\"2026-01-01T00:00:00Z\"'"

# --- Test 5: stdin payload ---
seed_manifest
echo '{"score": 99}' | "$MERGE" --section quality_review --payload - --manifest "$WORK/project.json" --quiet
assert "stdin payload accepted" "python3 -c 'import json; d=json.load(open(\"$WORK/project.json\")); assert d[\"quality_review\"]=={\"score\": 99}'"
check "stdin payload accepted" "python3 -c 'import json; d=json.load(open(\"$WORK/project.json\")); assert d[\"quality_review\"]=={\"score\": 99}'"

# --- Test 6: unknown section rejected with exit 3 ---
seed_manifest
set +e
"$MERGE" --section bogus_section --payload "$WORK/payload.json" --manifest "$WORK/project.json" --quiet 2>/dev/null
EC=$?
set -e
assert "unknown section exits 3" "[ $EC -eq 3 ]"
check "unknown section exits 3" "[ $EC -eq 3 ]"

# --- Test 7: malformed payload rejected with exit 1 ---
seed_manifest
Expand All @@ -93,47 +80,47 @@ set +e
"$MERGE" --section red_team_audit --payload "$WORK/bad.json" --manifest "$WORK/project.json" --quiet 2>/dev/null
EC=$?
set -e
assert "malformed payload exits 1" "[ $EC -eq 1 ]"
check "malformed payload exits 1" "[ $EC -eq 1 ]"

# --- Test 8: malformed manifest rejected with exit 2 ---
echo 'not json' > "$WORK/project.json"
set +e
echo '{}' | "$MERGE" --section red_team_audit --payload - --manifest "$WORK/project.json" --quiet 2>/dev/null
EC=$?
set -e
assert "malformed manifest exits 2" "[ $EC -eq 2 ]"
check "malformed manifest exits 2" "[ $EC -eq 2 ]"

# --- Test 9: missing manifest rejected with exit 4 ---
rm -f "$WORK/project.json"
set +e
echo '{}' | "$MERGE" --section red_team_audit --payload - --manifest "$WORK/project.json" --quiet 2>/dev/null
EC=$?
set -e
assert "missing manifest exits 4" "[ $EC -eq 4 ]"
check "missing manifest exits 4" "[ $EC -eq 4 ]"

# --- Test 10: missing payload file rejected with exit 5 ---
seed_manifest
set +e
"$MERGE" --section red_team_audit --payload "$WORK/nonexistent.json" --manifest "$WORK/project.json" --quiet 2>/dev/null
EC=$?
set -e
assert "missing payload file exits 5" "[ $EC -eq 5 ]"
check "missing payload file exits 5" "[ $EC -eq 5 ]"

# --- Test 11: manifest with non-dict root (list) rejected with exit 2 ---
echo '[]' > "$WORK/project.json"
set +e
echo '{}' | "$MERGE" --section red_team_audit --payload - --manifest "$WORK/project.json" --quiet 2>/dev/null
EC=$?
set -e
assert "manifest with list root exits 2" "[ $EC -eq 2 ]"
check "manifest with list root exits 2" "[ $EC -eq 2 ]"

# --- Test 12: manifest with non-dict root (string) rejected with exit 2 ---
echo '"a string"' > "$WORK/project.json"
set +e
echo '{}' | "$MERGE" --section red_team_audit --payload - --manifest "$WORK/project.json" --quiet 2>/dev/null
EC=$?
set -e
assert "manifest with string root exits 2" "[ $EC -eq 2 ]"
check "manifest with string root exits 2" "[ $EC -eq 2 ]"

echo ""
echo "manifest-merge: $PASS/$TOTAL passed, $FAIL failed"
Expand Down
60 changes: 29 additions & 31 deletions test/test-version-classifier.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@

set -e

PASS=0
FAIL=0
TOTAL=0
. "$(dirname "$0")/test_helper.sh"

# Mirror of the case statement in setup and bin/idstack-doctor. Keep these
# patterns in lockstep with both files — if you change one, change all three.
Expand All @@ -30,7 +28,7 @@ classify_version() {
esac
}

check() {
check_version() {

@gemini-code-assist gemini-code-assist Bot Jun 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Now that test_helper.sh is sourced and provides a robust check function, we can simplify check_version and eliminate the duplicated test-tracking (TOTAL, PASS, FAIL) and reporting logic.

You can refactor check_version to leverage the shared check helper:

check_version() {
  check "$1 -> $2" "[ \"\$(classify_version \"$1\")\" = \"$2\" ]"
}

This keeps the test cases clean while fully centralizing the test execution and counter logic.

TOTAL=$((TOTAL + 1))
local version="$1"
local expected="$2"
Expand All @@ -49,41 +47,41 @@ echo "test-version-classifier"
echo ""

# Legacy versions that ever shipped.
check "0.1.0" legacy
check "0.5.0" legacy
check "1.0.0" legacy
check "1.9.0" legacy
check "2.0.0" legacy
check "2.0.0.0" legacy
check "2.0.0.1" legacy
check_version "0.1.0" legacy
check_version "0.5.0" legacy
check_version "1.0.0" legacy
check_version "1.9.0" legacy
check_version "2.0.0" legacy
check_version "2.0.0.0" legacy
check_version "2.0.0.1" legacy

# Modern versions — including multi-digit components that broke the
# previous patterns.
check "2.0.1.0" skip
check "2.0.1.5" skip
check "2.0.10.0" skip
check "2.0.99.0" skip
check "2.1.0.0" skip
check "2.4.0.0" skip
check "2.10.0.0" skip
check "2.99.0.0" skip
check "3.0.0.0" skip
check "9.0.0.0" skip
check "10.0.0.0" skip
check "19.0.0.0" skip
check "100.0.0.0" skip
check_version "2.0.1.0" skip
check_version "2.0.1.5" skip
check_version "2.0.10.0" skip
check_version "2.0.99.0" skip
check_version "2.1.0.0" skip
check_version "2.4.0.0" skip
check_version "2.10.0.0" skip
check_version "2.99.0.0" skip
check_version "3.0.0.0" skip
check_version "9.0.0.0" skip
check_version "10.0.0.0" skip
check_version "19.0.0.0" skip
check_version "100.0.0.0" skip

# Future major versions where the major itself is multi-digit but does not
# start with 1. Caught by Gemini on PR #21 — the previous 1[0-9]* arm
# missed these and silently fell through to "unknown". Now classified by
# [1-9][0-9]*.
check "20.0.0.0" skip
check "21.5.0" skip
check "25.99.0" skip
check "29.0.0" skip
check "30.0.0" skip
check "200.0.0" skip
check "999.0.0" skip
check_version "20.0.0.0" skip
check_version "21.5.0" skip
check_version "25.99.0" skip
check_version "29.0.0" skip
check_version "30.0.0" skip
check_version "200.0.0" skip
check_version "999.0.0" skip

echo ""
echo " $PASS/$TOTAL passed"
Expand Down
16 changes: 16 additions & 0 deletions test/test_helper.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#!/usr/bin/env bash

PASS=0
FAIL=0
TOTAL=0

check() {
TOTAL=$((TOTAL + 1))
if eval "$2" >/dev/null 2>&1; then
PASS=$((PASS + 1))
echo " PASS: $1"
else
FAIL=$((FAIL + 1))
echo " FAIL: $1"
fi
Comment on lines +9 to +15

@gemini-code-assist gemini-code-assist Bot Jun 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

When a test fails, it is extremely helpful to see the actual error output or stdout of the failed command to diagnose the issue immediately. Currently, both stdout and stderr are completely silenced via >/dev/null 2>&1.

We can capture the output and print it indented only when the command fails, without cluttering the output of passing tests.

Suggested change
if eval "$2" >/dev/null 2>&1; then
PASS=$((PASS + 1))
echo " PASS: $1"
else
FAIL=$((FAIL + 1))
echo " FAIL: $1"
fi
local out
if out=$(eval "$2" 2>&1); then
PASS=$((PASS + 1))
echo " PASS: $1"
else
FAIL=$((FAIL + 1))
echo " FAIL: $1"
if [ -n "$out" ]; then
echo "$out" | sed 's/^/ /'
fi
fi

}