Skip to content

Commit b13d801

Browse files
committed
ci: derive the kinds from the matrix, normalize paths, lint both template spellings
Three fixes from review on 5b7e8b6. The kind list was hardcoded in three loops with nothing tying it to the jobs. Adding a fourth job with its matrix key and its MATRIX_KEYS entry while missing one of those loops wrote no output line for it, and `!= '[]'` is true for the empty string — so that job would start and die in fromJSON(''), the exact failure emit_all's comment describes. The loops now come from `jq -r 'keys_unsorted[]'` over the matrix file, and refuse to emit anything if that yields nothing. The guard's kind check became bidirectional to close the other direction: a kind a job reads but the file does not have is now reported, not just a key no job consumes. `directories` values are normalized when collected. Dependabot resolves "/examples/fastapi/app/" and "/examples/fastapi/app" to the same manifest, but comparing verbatim reported the entry as pointing at no manifest *and* the manifest as having no entry — two contradictory problems for a config that works, the failure already fixed here for the singular `directory` key and for globs. Stripping only slashes leaves glob patterns alone, and the duplicate check now also sees `/examples/x` against `/examples/x/` as the collision Dependabot rejects the file over. The validate job lints template.yml as well as template.yaml. examples/sinatra and examples/go-http-zip use the .yml spelling, so 43 of 45 templates were linted and those two were not — while examples-verified claimed "template validation only" for them. Both have Dependabot entries and no matrix entry, so that claim was the only thing their pull requests were going to get, and it was false. Exercised: a trailing-slash entry now passes; a slashed and unslashed pair of the same directory is reported as a duplicate; a matrix missing a kind a job reads, and a matrix with a kind no job reads, both fail; a fourth kind in the file gets its output line; an unreadable matrix refuses rather than emitting an empty selection; and the twelve-state guard suite plus three selector paths still behave.
1 parent 5b7e8b6 commit b13d801

3 files changed

Lines changed: 43 additions & 6 deletions

File tree

.github/scripts/check-example-config.sh

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,9 +124,20 @@ for update in config["updates"]:
124124
# but the singular is the canonical form for one directory and is what someone
125125
# adding an entry is likely to reach for — reading only the plural would report
126126
# their manifest as unclaimed and tell them to add an entry that is already there.
127-
directories = list(update.get("directories") or [])
127+
# Normalized on collection: Dependabot resolves "/examples/fastapi/app/" and
128+
# "/examples/fastapi/app" to the same manifest, but comparing the strings verbatim
129+
# reported the first as an entry with no manifest *and* the manifest as having no
130+
# entry — two contradictory problems for a config that works, which is the failure
131+
# already fixed here twice, for the singular `directory` key and for globs. Stripping
132+
# only slashes leaves glob patterns alone. It also lets the duplicate check see
133+
# `/examples/x` and `/examples/x/` as the collision Dependabot rejects the file over.
134+
def normalize(value):
135+
stripped = value.strip("/")
136+
return "/" + stripped if stripped else "/"
137+
138+
directories = [normalize(d) for d in (update.get("directories") or [])]
128139
if "directory" in update:
129-
directories.append(update["directory"])
140+
directories.append(normalize(update["directory"]))
130141
131142
where = f"{ecosystem} {directories}"
132143
@@ -305,12 +316,21 @@ MATRIX_KEYS = {
305316
306317
matrix = json.load(open(matrix_path))
307318
319+
# Both directions. A key no job reads is dead weight; a kind a job reads that the file
320+
# does not have is worse — select-examples.sh derives its loops from this file, so that
321+
# job would get no output line, and `!= '[]'` is true for the empty string.
308322
unknown_kinds = sorted(set(matrix) - set(MATRIX_KEYS))
309323
if unknown_kinds:
310324
problems.append(
311325
"%s has kinds no job consumes: %s" % (matrix_path, ", ".join(unknown_kinds))
312326
)
313327
328+
missing_kinds = sorted(set(MATRIX_KEYS) - set(matrix))
329+
if missing_kinds:
330+
problems.append(
331+
"%s is missing kinds a job reads: %s" % (matrix_path, ", ".join(missing_kinds))
332+
)
333+
314334
for kind, entries in matrix.items():
315335
expected = MATRIX_KEYS.get(kind)
316336
if expected is None:

.github/scripts/select-examples.sh

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,26 @@ set -euo pipefail
2727

2828
MATRIX="$(dirname "$0")/../example-matrix.json"
2929

30+
# The kinds come from the matrix file, not from three hardcoded lists. Adding a fourth job
31+
# and its matrix key while forgetting one of those lists wrote no output line for it at
32+
# all, and `!= '[]'` is true for the empty string — so the job would start and die in
33+
# fromJSON(''), which is the failure this file's emit_all comment describes. The guard
34+
# asserts the other direction (a kind a job reads must exist in the file), so between them
35+
# every key gets a line and every line has a consumer.
36+
mapfile -t KINDS < <(jq -r 'keys_unsorted[]' "$MATRIX")
37+
if [[ ${#KINDS[@]} -eq 0 ]]; then
38+
echo "No kinds in $MATRIX; refusing to emit an empty selection." >&2
39+
exit 1
40+
fi
41+
3042
# Assign before echoing, so a jq failure is the command's status rather than an
3143
# argument to echo: `echo "x=$(jq ...)"` returns echo's 0 even when jq dies, and
3244
# set -e never fires. That wrote `image=` to $GITHUB_OUTPUT and reported success — and
3345
# an empty value is worse than a failure, because `!= '[]'` is true for it, so the test
3446
# jobs would run and die in fromJSON('') with an error unrelated to the real cause.
3547
emit_all() {
3648
local kind matrix
37-
for kind in image zip stream; do
49+
for kind in "${KINDS[@]}"; do
3850
# `has` rather than a bare `.$kind`: jq prints the literal `null` and exits 0 for a
3951
# missing key, so a renamed top-level key in the matrix file wrote `stream=null`,
4052
# which `!= '[]'` reads as truthy — test-stream would start and die in
@@ -112,7 +124,7 @@ example_paths="$(grep -oE '^examples/[^/]+/' <<<"$changed" || true)"
112124
# `if: ... != '[]'` guards in examples.yaml skip the test jobs and the workflow is green.
113125
if [[ -z "$example_paths" ]]; then
114126
echo "No example changed: nothing to verify."
115-
for kind in image zip stream; do
127+
for kind in "${KINDS[@]}"; do
116128
echo "$kind=[]" >>"$GITHUB_OUTPUT"
117129
done
118130
exit 0
@@ -138,7 +150,7 @@ if [[ -n "$uncovered" ]]; then
138150
fi
139151
fi
140152

141-
for kind in image zip stream; do
153+
for kind in "${KINDS[@]}"; do
142154
# Same has() assertion as emit_all: without it a renamed top-level key fails here with
143155
# jq's bare "Cannot iterate over null", naming neither the file nor the key, while the
144156
# push path says which key is missing. Loud is not the same as diagnostic.

.github/workflows/examples.yaml

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,12 @@ jobs:
106106
- name: Validate all SAM templates
107107
run: |
108108
failed=0
109-
for template in $(find examples -maxdepth 2 -name "template.yaml" | sort); do
109+
# Both spellings: examples/sinatra and examples/go-http-zip use template.yml,
110+
# so a .yaml-only find left them unlinted while examples-verified still claimed
111+
# "template validation only" for them — and both have Dependabot entries and no
112+
# matrix entry, so that claim is the only thing their pull requests get.
113+
for template in $(find examples -maxdepth 2 \
114+
\( -name "template.yaml" -o -name "template.yml" \) | sort); do
110115
dir=$(dirname "$template")
111116
echo "Validating $dir..."
112117
if ! sam validate --template "$template" --lint 2>&1; then

0 commit comments

Comments
 (0)