Skip to content

Coverage thresholds and @max_ms benchmark limits break on non-integer values #879

Description

@Chemaclass

🤔 Background

Two more instances of the class fixed in #871/#873/#875/#877: a value reaches [ -ge ] / [ -le ], which errors (status 2) rather than returning false on a non-integer operand, so the comparison result is silently wrong.

1. BASHUNIT_COVERAGE_THRESHOLD_LOW / _HIGH

Documented in docs/coverage.md and docs/configuration.md, env-only (no CLI flag), never validated. bashunit::coverage::get_coverage_class (src/coverage.sh:159) compares with [ -ge ]:

$ BASHUNIT_COVERAGE_THRESHOLD_HIGH=abc  # reaching the call site
src/coverage.sh: line 159: [: abc: integer expression expected
medium        # 90% coverage, should be "high"

Two symptoms: a raw shell error leaks into the coverage report, and every file is mis-bucketed — the errored [ ] falls through to the elif, so 90% is classed medium instead of high. Reachable from 6 call sites in the text and HTML report writers.

Note on reproducing it: the repo's own .env (and .env.example, which CI copies) lists both names with an empty value. Because .env is loaded with set -o allexport, that empty listing overrides an exported value, so the bug is not reproducible from inside a repo checkout — you need --skip-env-file or an environment without those listings. This is #865, and it is very likely why the bug went unnoticed.

2. @max_ms benchmark annotation with a decimal

The annotation parser explicitly accepts decimals ([0-9.][0-9.]*, src/benchmark.sh:40) and the measured average can itself be fractional, but the comparison at src/benchmark.sh:146 uses integer [ -le ]:

$ cat x_bench.sh
# @max_ms=1000.5
function bench_quick() { :; }

$ ./bashunit bench .
Name                        Revs    Its    Avg(ms)       Status
./src/benchmark.sh: line 146: [: 1000.5: integer expression expected
bench_quick                    1      1          8     > 1000.5

The row renders as failing (>) regardless of the actual average — 8ms is plainly under 1000.5 — because the errored [ ] returns non-zero either way. A benchmark threshold with a decimal is therefore always reported as breached.

Suggested fix

  • Thresholds: extend bashunit::main::validate_config_or_exit to run them through the existing require_non_negative_int_or_exit.
  • @max_ms: compare with a float-tolerant helper, mirroring the existing bashunit::math::calculate bc → awk fallback chain already used elsewhere in that file.

Found by the codebase-quality sweep.

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

Status
No status

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions