Skip to content

🧪 Add tests for idstack-learnings-delete #41

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

Merged
savvides merged 2 commits into from
Aug 6, 2026
Merged

🧪 Add tests for idstack-learnings-delete #41

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f61c436
🧪 Add tests for idstack-learnings-delete
google-labs-jules[bot] Jun 6, 2026
37c4593
Merge branch 'main' into add-tests-idstack-learnings-delete-277366469…
savvides Aug 6, 2026
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
25 changes: 25 additions & 0 deletions test/integration-test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,31 @@ check "missing {{PREAMBLE}} placeholder fails dry-run and generation (sandbox)"
check "real tree untouched by this suite" \
"[ \"\$(git -C '$IDSTACK_DIR' status --porcelain 2>/dev/null || true)\" = \"\$TREE_BEFORE\" ]"

echo ""

# --- idstack-learnings-delete ---
echo "## idstack-learnings-delete"

check "returns error if no arguments provided" \
"! $IDSTACK_DIR/bin/idstack-learnings-delete"

check "returns error if no file exists" \
"rm -f .idstack/learnings.jsonl && ! $IDSTACK_DIR/bin/idstack-learnings-delete somekey"

# Setup data for delete tests
$IDSTACK_DIR/bin/idstack-learnings-log '{"skill":"test","type":"fact","key":"key1","insight":"one"}'
$IDSTACK_DIR/bin/idstack-learnings-log '{"skill":"test","type":"fact","key":"key2","insight":"two"}'
$IDSTACK_DIR/bin/idstack-learnings-log '{"skill":"test","type":"fact","key":"key1","insight":"three"}'

check "returns error if key not found" \
"! $IDSTACK_DIR/bin/idstack-learnings-delete missingkey"
Comment on lines +186 to +187

@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 delete operation fails (e.g., because the key was not found), it is important to verify that the underlying data file remains unmodified and is not corrupted or cleared.

We can enhance this test by asserting that the line count of .idstack/learnings.jsonl remains exactly 3 after the failed delete attempt.

Suggested change
check "returns error if key not found" \
"! $IDSTACK_DIR/bin/idstack-learnings-delete missingkey"
check "returns error if key not found" \
"! $IDSTACK_DIR/bin/idstack-learnings-delete missingkey && [ \$(wc -l < .idstack/learnings.jsonl | tr -d ' ') -eq 3 ]"


check "deletes the most recent entry with the key when there are duplicates" \
"$IDSTACK_DIR/bin/idstack-learnings-delete key1 && python3 -c \"import json,sys; lines=[json.loads(l) for l in open('.idstack/learnings.jsonl')]; assert len(lines)==2 and lines[0]['key']=='key1' and lines[0]['insight']=='one' and lines[1]['key']=='key2'\""

check "deletes a specific learning" \
"$IDSTACK_DIR/bin/idstack-learnings-delete key2 && python3 -c \"import json,sys; lines=[json.loads(l) for l in open('.idstack/learnings.jsonl')]; assert len(lines)==1 and lines[0]['key']=='key1'\""
Comment on lines +192 to +193

@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

This suggestion introduces three improvements:

  1. Robustness: Verifies the insight field of the remaining entry to ensure the correct item was preserved.
  2. Edge Case Coverage: Adds a new test case to verify that deleting the last remaining entry in the file works correctly and leaves the file empty (or deleted), which is a common edge case for file-based deletion scripts.
  3. Cleanliness: Removes the unused sys import from the Python one-liner.
Suggested change
check "deletes a specific learning" \
"$IDSTACK_DIR/bin/idstack-learnings-delete key2 && python3 -c \"import json,sys; lines=[json.loads(l) for l in open('.idstack/learnings.jsonl')]; assert len(lines)==1 and lines[0]['key']=='key1'\""
check "deletes a specific learning" \
"$IDSTACK_DIR/bin/idstack-learnings-delete key2 && python3 -c \"import json; lines=[json.loads(l) for l in open('.idstack/learnings.jsonl')]; assert len(lines)==1 and lines[0]['key']=='key1' and lines[0]['insight']=='one'\""
check "deletes the last remaining learning" \
"$IDSTACK_DIR/bin/idstack-learnings-delete key1 && [ ! -s .idstack/learnings.jsonl ]"


echo ""
echo "Results: $PASS/$TOTAL passed, $FAIL failed"
[ "$FAIL" -eq 0 ] && exit 0 || exit 1
Loading