diff --git a/CHANGELOG.md b/CHANGELOG.md index 983588f3..9bfa7591 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - `--verbose` warns on Bash 3.x that coverage does not count lines run inside a subshell, so a percentage that reads lower there than on Bash 4+ explains itself (#1112) ### Changed +- `bashunit learn` generates starter files that are valid bash. A function body of only TODO comments is a syntax error, so 7 of the 10 lesson templates did not parse and a learner running the lesson saw `syntax error near unexpected token '}'` from a file bashunit itself wrote. Lesson verification also runs with `--fail-on-risky`, so an untouched template no longer completes a lesson — a test with no assertions is risky, and risky exits 0 (#1256) - The coverage HTML report handles filenames containing `|`, `<` or `&`. Rows were joined and split on `|`, so `src/a|b.sh` truncated to `a` in the index while the coverage numbers stayed right, and filenames went into the markup unescaped — `ac.sh` was parsed as a tag and leaked into the document. The escaper now lives in `src/util/str.sh`, shared with the test report instead of duplicated (#1254) - The HTML report summary counts risky and flaky tests. A run with a risky test showed `2 total` against categories summing to 1, with nothing on the page saying where the second test went — the row was there with its own CSS class, but the summary never counted it. The console and the Markdown report both report it (#1252) - The HTML report says **why** a test failed. It listed name, status and duration only, while JUnit, JSON, TAP and Markdown all carry the message — and HTML is the format opened in a browser to find out what broke. A `Failures` section now gives each failure its name, `file:line` and message; a green run gains nothing (#1251) diff --git a/src/learn/lessons/basics.sh b/src/learn/lessons/basics.sh index b182d202..aaaac59a 100644 --- a/src/learn/lessons/basics.sh +++ b/src/learn/lessons/basics.sh @@ -50,6 +50,7 @@ EOF function test_bashunit_works() { # TODO: Use assert_same to check if "hello" equals "hello" # Hint: assert_same "expected" "actual" + : }' bashunit::learn::create_example_file "$test_file" "$template" diff --git a/src/learn/lessons/challenge.sh b/src/learn/lessons/challenge.sh index 0c7411b2..d115ea30 100644 --- a/src/learn/lessons/challenge.sh +++ b/src/learn/lessons/challenge.sh @@ -69,14 +69,17 @@ function set_up() { function tear_down() { # TODO: Clean up test files + : } function test_successful_backup() { # TODO: Test backup creation + : } function test_backup_failure_when_source_missing() { # TODO: Test failure case + : } # Add more tests as needed: diff --git a/src/learn/lessons/data_providers.sh b/src/learn/lessons/data_providers.sh index 51cc6c3d..f187e8ac 100644 --- a/src/learn/lessons/data_providers.sh +++ b/src/learn/lessons/data_providers.sh @@ -86,22 +86,26 @@ function set_up() { function data_provider_valid_emails() { # TODO: Echo valid email addresses, one per line # Example: echo "user@example.com" + : } function test_valid_emails() { # $1 contains the email from data provider # TODO: Assert is_valid_email succeeds # Hint: assert_successful_code "is_valid_email \"$1\"" + : } function data_provider_invalid_emails() { # TODO: Echo invalid email addresses, one per line # Example: echo "not-an-email" + : } function test_invalid_emails() { # TODO: Assert is_valid_email fails # Hint: assert_general_error "is_valid_email \"$1\"" + : }' bashunit::learn::create_example_file "$test_file" "$template" diff --git a/src/learn/lessons/exit_codes.sh b/src/learn/lessons/exit_codes.sh index dbebb1c3..f14b12d5 100644 --- a/src/learn/lessons/exit_codes.sh +++ b/src/learn/lessons/exit_codes.sh @@ -99,11 +99,13 @@ function tear_down() { function test_existing_file_returns_success() { # TODO: Assert check_file succeeds with TEST_FILE # Hint: assert_successful_code "check_file '\''$TEST_FILE'\''" + : } function test_missing_file_returns_127() { # TODO: Assert check_file returns exit code 127 for missing file # Hint: assert_exit_code 127 "check_file '\''/nonexistent/file'\''" + : }' bashunit::learn::create_example_file "$test_file" "$template" diff --git a/src/learn/lessons/functions.sh b/src/learn/lessons/functions.sh index d3acdd6f..9eec1b00 100644 --- a/src/learn/lessons/functions.sh +++ b/src/learn/lessons/functions.sh @@ -69,18 +69,21 @@ EOF function set_up() { # TODO: Source calculator.sh from parent directory # Hint: source ../calculator.sh + : } function test_add_positive_numbers() { # TODO: Test that add 2 3 returns "5" # Hint: result=$(add 2 3) # Hint: assert_same "5" "$result" + : } function test_add_negative_numbers() { # TODO: Test that add -2 -3 returns "-5" # Hint: result=$(add -2 -3) # Hint: assert_same "-5" "$result" + : }' bashunit::learn::create_example_file "$test_file" "$template" diff --git a/src/learn/lessons/lifecycle.sh b/src/learn/lessons/lifecycle.sh index e22ae72c..3865abd7 100644 --- a/src/learn/lessons/lifecycle.sh +++ b/src/learn/lessons/lifecycle.sh @@ -66,19 +66,23 @@ function set_up() { # Create a temp file before each test # TODO: export TEST_FILE="/tmp/test_$$" # TODO: echo "test content" > "$TEST_FILE" + : } function tear_down() { # Clean up after each test # TODO: rm -f "$TEST_FILE" + : } function test_file_exists() { # TODO: assert_file_exists "$TEST_FILE" + : } function test_file_has_content() { # TODO: assert_file_contains "test content" "$TEST_FILE" + : }' bashunit::learn::create_example_file "$test_file" "$template" diff --git a/src/learn/lessons/scripts.sh b/src/learn/lessons/scripts.sh index 3afcdd73..57168a50 100644 --- a/src/learn/lessons/scripts.sh +++ b/src/learn/lessons/scripts.sh @@ -70,6 +70,7 @@ function test_default_greeting() { # TODO: Assert output contains "Hello, World!" # Hint: assert_contains "Hello, World!" "$output" + : } function test_custom_greeting() { @@ -78,6 +79,7 @@ function test_custom_greeting() { # TODO: Assert output contains "Hello, Alice!" # Hint: assert_contains "Hello, Alice!" "$output" + : }' bashunit::learn::create_example_file "$test_file" "$template" diff --git a/src/learn/session.sh b/src/learn/session.sh index 77127526..b3a28cef 100644 --- a/src/learn/session.sh +++ b/src/learn/session.sh @@ -53,7 +53,11 @@ function bashunit::learn::run_lesson_test() { echo "${_BASHUNIT_COLOR_BOLD}Running your test...${_BASHUNIT_COLOR_DEFAULT}" echo "" - if "$BASHUNIT_ROOT_DIR/bashunit" "$test_file" --simple; then + # --fail-on-risky, or an untouched template completes the lesson: a test whose + # body is still only TODO comments records no assertions, which is *risky*, + # and risky exits 0 by default. The learner is told "Excellent!" for work they + # have not done (#1256). + if "$BASHUNIT_ROOT_DIR/bashunit" "$test_file" --simple --fail-on-risky; then echo "" printf "%s%s✓ Excellent! Lesson %s completed!%s\n" \ "$_BASHUNIT_COLOR_PASSED" "$_BASHUNIT_COLOR_BOLD" "$lesson_number" "$_BASHUNIT_COLOR_DEFAULT" diff --git a/tests/unit/project/learn_templates_test.sh b/tests/unit/project/learn_templates_test.sh new file mode 100644 index 00000000..65355aa6 --- /dev/null +++ b/tests/unit/project/learn_templates_test.sh @@ -0,0 +1,102 @@ +#!/usr/bin/env bash + +# `bashunit learn` writes a starter file for each lesson. Seven of the ten +# templates did not parse: a function body of only TODO comments is a bash +# syntax error, and there were 28 such bodies. A learner who generated the file +# and ran the lesson got +# +# syntax error near unexpected token `}' +# +# from a file bashunit itself had written -- which says nothing about the +# lesson and is a newcomer's first contact with the tool (#1256). +# +# Templates are heredoc-free string literals, so nothing else checks them: +# ShellCheck sees a string, and no test sourced one. This does. + +function set_up_before_script() { + ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/../../.." && pwd)" +} + +# Every `local template='…'` block in the lesson files, NUL-separated so an +# embedded newline does not split one template into several. +# +# awk, not perl: Alpine ships without perl, and the snapshot placeholder tests +# already skip there for that reason -- but template validity does not depend +# on the platform, so this has to run everywhere rather than skip. +# +# The block ends at the first line whose last character is the closing quote. +# That is safe because no line *inside* a template ends in one; the only such +# line in these files sits in the `cat <<'EOF'` lesson text, outside any +# template. Checked against a perl extractor: both find 10 templates and agree +# on which parse. +function _lesson_templates() { # $1 = lessons dir + local file + for file in "$1"/*.sh; do + awk ' + index($0, "local template=\047") { + intpl = 1 + sub(/^.*local template=\047/, "") + } + intpl { + if (substr($0, length($0), 1) == "\047") { + sub(/\047$/, "") + print + printf "%c", 0 + intpl = 0 + next + } + print + } + ' "$file" + done +} + +function test_every_lesson_template_is_valid_bash() { + local invalid="" + local tpl + local tmp + tmp="$(bashunit::temp_file)" + + while IFS= read -r -d '' tpl; do + printf '%s\n' "$tpl" >"$tmp" + if ! bash -n "$tmp" 2>/dev/null; then + invalid="$invalid$(printf '%s\n' "$tpl" | "$GREP" -m1 'function' || true) " + fi + done < <(_lesson_templates "$ROOT_DIR/src/learn/lessons") + + assert_empty "$invalid" +} + +# A check that cannot fail proves nothing: the templates are expected to parse, +# so without this the scan above would keep passing if the extraction silently +# stopped matching. +function test_the_scan_flags_a_template_that_does_not_parse() { + local dir + dir="$(bashunit::temp_dir)" + { + printf '%s\n' '#!/usr/bin/env bash' + printf '%s\n' 'function bashunit::learn::lesson_probe() {' + printf '%s\n' " local template='#!/usr/bin/env bash" + printf '%s\n' 'function test_probe() {' + printf '%s\n' ' # only a comment, which bash rejects' + printf '%s\n' "}'" + printf '%s\n' '}' + } >"$dir/probe.sh" + + local found=false + local tpl tmp + tmp="$(bashunit::temp_file)" + while IFS= read -r -d '' tpl; do + printf '%s\n' "$tpl" >"$tmp" + bash -n "$tmp" 2>/dev/null || found=true + done < <(_lesson_templates "$dir") + + assert_same "true" "$found" +} + +# The other half of #1256: a template that parses but has no assertions must not +# complete a lesson. `run_lesson_test` treats exit 0 as success, and a test +# without assertions is risky, which exits 0 unless --fail-on-risky is passed. +function test_the_lesson_runner_fails_a_test_with_no_assertions() { + assert_file_contains "$ROOT_DIR/src/learn/session.sh" "--fail-on-risky" +}