Skip to content
Open
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,31 @@ check "dry-run detects stale SKILL.md" \
check "regenerate fixes staleness" \
"$IDSTACK_DIR/bin/idstack-gen-skills && $IDSTACK_DIR/bin/idstack-gen-skills --dry-run"

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 +133 to +134

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 +139 to +140

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