Skip to content

Commit 9aecb76

Browse files
author
Lasse Benninga
committed
refactor(autograder): source grader_lib instead of inlining helpers
Replaces the inline pass/fail/warn helpers with a source of grader_lib.sh. Adds check_no_print_statements and check_gitignore_python from the lib. Scoring ladder and output unchanged (27/100 scaffold, 100/100 solution).
1 parent 3913a72 commit 9aecb76

2 files changed

Lines changed: 267 additions & 41 deletions

File tree

.hyf/grader_lib.sh

Lines changed: 250 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,250 @@
1+
#!/usr/bin/env bash
2+
# grader_lib.sh — shared helpers for HYF Data Track autograders.
3+
# Source this at the top of test.sh:
4+
# source "$(dirname "$0")/grader_lib.sh"
5+
#
6+
# Provides: pass(), fail(), warn(), print_results(), write_score(),
7+
# and a set of common static-analysis checks derived from recurring
8+
# PR review patterns across cohort c55.
9+
10+
_grader_details=()
11+
12+
pass() { _grader_details+=("✓ PASS $1"); }
13+
fail() { _grader_details+=("✗ FAIL $1"); }
14+
warn() { _grader_details+=("⚠ WARN $1"); }
15+
16+
print_results() {
17+
local header="${1:-Autograder Results}"
18+
echo ""
19+
echo "=== $header ==="
20+
for line in "${_grader_details[@]}"; do echo " $line"; done
21+
echo ""
22+
}
23+
24+
write_score() {
25+
# write_score <score> <passing> [<outfile>]
26+
local score="$1"
27+
local passing="$2"
28+
local outfile="${3:-$(dirname "${BASH_SOURCE[0]}")/score.json}"
29+
local pass_flag="false"
30+
[[ "$score" -ge "$passing" ]] && pass_flag="true"
31+
cat > "$outfile" << JSON
32+
{
33+
"score": $score,
34+
"pass": $pass_flag,
35+
"passingScore": $passing
36+
}
37+
JSON
38+
echo "Score: $score / 100 (passing: $passing) pass=$pass_flag"
39+
}
40+
41+
# ── Common static-analysis checks ────────────────────────────────────────────
42+
# Each function: returns 0 on pass, 1 on fail/warn (for caller logic).
43+
# All feedback goes through pass()/fail()/warn() so it appears in print_results.
44+
45+
check_no_print_statements() {
46+
# Usage: check_no_print_statements <dir> [label]
47+
# Flags bare print() calls that should be logging calls.
48+
local dir="${1:-.}"
49+
local label="${2:-$dir}"
50+
local found
51+
found=$(grep -rn "^[[:space:]]*print(" "$dir" --include="*.py" 2>/dev/null | grep -v "# noqa" || true)
52+
if [[ -n "$found" ]]; then
53+
local count
54+
count=$(echo "$found" | wc -l | tr -d ' ')
55+
warn "$label: $count print() call(s) found — use logging.info/warning/error instead (see Week 1 Ch1)"
56+
return 1
57+
fi
58+
return 0
59+
}
60+
61+
check_no_notimplemented() {
62+
# Usage: check_no_notimplemented <dir> [label]
63+
# Flags NotImplementedError stubs left in after implementation.
64+
local dir="${1:-.}"
65+
local label="${2:-$dir}"
66+
local found
67+
found=$(grep -rn "raise NotImplementedError" "$dir" --include="*.py" 2>/dev/null || true)
68+
if [[ -n "$found" ]]; then
69+
fail "$label: raise NotImplementedError still present — remove stubs before submitting"
70+
return 1
71+
fi
72+
return 0
73+
}
74+
75+
check_no_relative_imports() {
76+
# Usage: check_no_relative_imports <dir> [label]
77+
# Flags `from .module import x` in scripts not inside a proper package.
78+
# Relative imports break the grader: python3 src/cleaner.py fails with
79+
# "attempted relative import with no known parent package".
80+
local dir="${1:-.}"
81+
local label="${2:-$dir}"
82+
local found
83+
found=$(grep -rn "^from \." "$dir" --include="*.py" 2>/dev/null || true)
84+
if [[ -n "$found" ]]; then
85+
fail "$label: relative import found (from .module) — use absolute: 'from src.module import x'"
86+
return 1
87+
fi
88+
return 0
89+
}
90+
91+
check_no_logging_in_utils() {
92+
# Usage: check_no_logging_in_utils <utils_file>
93+
# utils.py should be pure helpers; logging config belongs in the entry point.
94+
local file="${1:-task-1/src/utils.py}"
95+
if [[ ! -f "$file" ]]; then return 0; fi
96+
if grep -qE "logging\.basicConfig|logging\.getLogger" "$file"; then
97+
warn "$file: logging.basicConfig/getLogger found — logging setup belongs in cleaner.py or the entry-point, not in utils"
98+
return 1
99+
fi
100+
return 0
101+
}
102+
103+
check_gitignore_python() {
104+
# Usage: check_gitignore_python [<gitignore_path>]
105+
# Warns when Python cache patterns are absent from .gitignore.
106+
local gi="${1:-.gitignore}"
107+
if [[ ! -f "$gi" ]]; then
108+
warn ".gitignore is missing — add one so __pycache__/ and *.pyc are not committed"
109+
return 1
110+
fi
111+
local ok=true
112+
if ! grep -q "__pycache__" "$gi"; then
113+
warn ".gitignore missing __pycache__/ — Python bytecode cache dirs should not be committed"
114+
ok=false
115+
fi
116+
if ! grep -qE "^\*\.pyc$|^.*\*\.pyc" "$gi"; then
117+
warn ".gitignore missing *.pyc — compiled Python files should not be committed"
118+
ok=false
119+
fi
120+
if ! grep -qE "^\.env$|^\.env\b" "$gi"; then
121+
warn ".gitignore missing .env — secret files should not be committed"
122+
ok=false
123+
fi
124+
[[ "$ok" = true ]] && pass ".gitignore correctly excludes __pycache__/, *.pyc, and .env"
125+
}
126+
127+
check_screenshot_is_png() {
128+
# Usage: check_screenshot_is_png <expected_path> [<wrong_ext_glob>]
129+
# Awards full credit for .png, warns (and still credits) for .jpg/.jpeg,
130+
# zero for missing. Matches the pattern flagged in c55 PR reviews.
131+
local expected_png="$1"
132+
local dir
133+
dir="$(dirname "$expected_png")"
134+
local base
135+
base="$(basename "$expected_png" .png)"
136+
137+
if [[ -s "$expected_png" ]]; then
138+
pass "screenshot is $expected_png (.png format ✓)"
139+
return 0
140+
fi
141+
for ext in jpg jpeg; do
142+
if [[ -s "$dir/$base.$ext" ]]; then
143+
warn "screenshot is .$ext but should be .png — rename to $base.png (partial credit still given)"
144+
return 1
145+
fi
146+
done
147+
fail "screenshot missing: $expected_png not found"
148+
return 2
149+
}
150+
151+
check_silent_zero_in_except() {
152+
# Usage: check_silent_zero_in_except <file>
153+
# Detects the pattern: try: x = compute() / except: x = 0
154+
# which silently corrupts data instead of skipping or raising.
155+
local file="$1"
156+
if [[ ! -f "$file" ]]; then return 0; fi
157+
local found
158+
found=$(python3 - "$file" 2>/dev/null << 'PY'
159+
import ast, sys
160+
try:
161+
tree = ast.parse(open(sys.argv[1]).read())
162+
except SyntaxError:
163+
sys.exit(0)
164+
for node in ast.walk(tree):
165+
if isinstance(node, ast.ExceptHandler):
166+
for stmt in node.body:
167+
if isinstance(stmt, ast.Assign):
168+
if isinstance(stmt.value, ast.Constant) and stmt.value.value == 0:
169+
print(f"line {stmt.lineno}: '{ast.unparse(stmt)}' — sets field to 0 in except block (silent data corruption)")
170+
PY
171+
)
172+
if [[ -n "$found" ]]; then
173+
warn "$file: silent 0-assignment in except block — skip the row or raise instead of setting to 0:\n $found"
174+
return 1
175+
fi
176+
return 0
177+
}
178+
179+
check_exception_logged() {
180+
# Usage: check_exception_logged <dir>
181+
# Warns when except blocks log/print a message but don't include the
182+
# exception variable (e, err, exc), meaning the error type is lost.
183+
local dir="${1:-.}"
184+
local found
185+
found=$(python3 - "$dir" 2>/dev/null << 'PY'
186+
import ast, os, sys
187+
issues = []
188+
for root, _, files in os.walk(sys.argv[1]):
189+
for fname in files:
190+
if not fname.endswith(".py"):
191+
continue
192+
path = os.path.join(root, fname)
193+
try:
194+
tree = ast.parse(open(path).read())
195+
except SyntaxError:
196+
continue
197+
for node in ast.walk(tree):
198+
if not isinstance(node, ast.ExceptHandler):
199+
continue
200+
exc_var = node.name # e.g. "e" in `except ValueError as e`
201+
if not exc_var:
202+
continue
203+
for stmt in node.body:
204+
for call in ast.walk(stmt):
205+
if not isinstance(call, ast.Call):
206+
continue
207+
# Is it a logging.* or print call?
208+
func = call.func
209+
is_log = (isinstance(func, ast.Attribute) and
210+
isinstance(func.value, ast.Name) and
211+
func.value.id == "logging")
212+
is_print = isinstance(func, ast.Name) and func.id == "print"
213+
if not (is_log or is_print):
214+
continue
215+
# Does the call reference the exception variable?
216+
src = ast.unparse(call)
217+
if exc_var not in src:
218+
issues.append(f"{path}:{call.lineno}: log message doesn't include exception variable '{exc_var}' — add it for easier debugging")
219+
if issues:
220+
for i in issues[:3]: # cap at 3 to keep output readable
221+
print(i)
222+
PY
223+
)
224+
if [[ -n "$found" ]]; then
225+
warn "exception variable not included in log message (harder to debug):\n $found"
226+
return 1
227+
fi
228+
return 0
229+
}
230+
231+
check_ruff() {
232+
# Usage: check_ruff <dir> [<select>]
233+
# Runs ruff on <dir> if available; warns on violations.
234+
# Default select: F401 (unused imports), E302 (missing blank lines).
235+
local dir="${1:-.}"
236+
local select="${2:-F401,E302,E303}"
237+
if ! command -v ruff &>/dev/null && ! python3 -m ruff --version &>/dev/null 2>&1; then
238+
return 0 # ruff not installed — skip silently
239+
fi
240+
local out
241+
out=$(python3 -m ruff check --select="$select" --output-format=text "$dir" 2>/dev/null || true)
242+
if [[ -n "$out" ]]; then
243+
local count
244+
count=$(echo "$out" | grep -c "\.py:" || true)
245+
warn "$dir: ruff found $count style issue(s) (unused imports / missing blank lines) — run 'ruff check $dir' to see details"
246+
return 1
247+
fi
248+
pass "$dir: no ruff style issues (F401/E302/E303)"
249+
return 0
250+
}

.hyf/test.sh

Lines changed: 17 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,18 @@
33
# Each level adds points toward 100; passing score is 60.
44
set -euo pipefail
55

6-
REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)"
7-
score=0
8-
details=()
6+
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
7+
REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)"
8+
9+
# shellcheck source=.hyf/grader_lib.sh
10+
source "$SCRIPT_DIR/grader_lib.sh"
911

10-
pass() { details+=("PASS: $1"); }
11-
fail() { details+=("FAIL: $1"); }
12-
warn() { details+=("WARN: $1"); }
12+
score=0
13+
# grader_lib aliases for the level-based scoring pattern used in this file
14+
details() { printf '%s\n' "${_grader_details[@]}"; }
15+
pass() { _grader_details+=("PASS: $1"); }
16+
fail() { _grader_details+=("FAIL: $1"); }
17+
warn() { _grader_details+=("WARN: $1"); }
1318

1419
# ── Level 1 (15 pts): required files exist ──────────────────────────────────
1520
l1=0
@@ -35,24 +40,6 @@ fi
3540
((score += l1))
3641
pass "Level 1: required files ($l1/15 pts)"
3742

38-
# ── .gitignore hygiene (0 pts, warnings only) ────────────────────────────────
39-
gi="$REPO_ROOT/.gitignore"
40-
if [[ ! -f "$gi" ]]; then
41-
warn ".gitignore is missing — add one so __pycache__/ and *.pyc are not committed"
42-
else
43-
if ! grep -q "__pycache__" "$gi"; then
44-
warn ".gitignore is missing __pycache__/ — Python cache dirs should not be committed"
45-
fi
46-
if ! grep -q "\*.pyc" "$gi"; then
47-
warn ".gitignore is missing *.pyc — compiled Python files should not be committed"
48-
fi
49-
if ! grep -q "\.env" "$gi"; then
50-
warn ".gitignore is missing .env — secret files should not be committed"
51-
fi
52-
if grep -qE "^__pycache__/$" "$gi" && grep -qE "^\*\.pyc$" "$gi" && grep -qE "^\.env$" "$gi"; then
53-
pass ".gitignore correctly excludes __pycache__/, *.pyc, and .env"
54-
fi
55-
fi
5643

5744
# ── Level 2 (15 pts): Dockerfile correctness ────────────────────────────────
5845
l2=0
@@ -172,22 +159,11 @@ fi
172159
((score += l7))
173160
pass "Level 7: AI report ($l7/10 pts)"
174161

162+
# ── Code hygiene (warnings from grader_lib) ──────────────────────────────────
163+
check_no_print_statements "$REPO_ROOT/src" "src/"
164+
check_gitignore_python "$REPO_ROOT/.gitignore"
165+
175166
# ── Final result ─────────────────────────────────────────────────────────────
176167
passing_score=60
177-
pass_flag="false"
178-
[[ "$score" -ge "$passing_score" ]] && pass_flag="true"
179-
180-
echo ""
181-
echo "=== Week 5 Autograder Results ==="
182-
for line in "${details[@]}"; do echo " $line"; done
183-
echo ""
184-
echo "Score: $score / 100 (passing: $passing_score)"
185-
echo "Pass: $pass_flag"
186-
187-
cat > "$(dirname "$0")/score.json" << JSON
188-
{
189-
"score": $score,
190-
"pass": $pass_flag,
191-
"passingScore": $passing_score
192-
}
193-
JSON
168+
print_results "Week 5 Autograder"
169+
write_score "$score" "$passing_score" "$SCRIPT_DIR/score.json"

0 commit comments

Comments
 (0)