-
-
Notifications
You must be signed in to change notification settings - Fork 107
Description
Parent Issue
Part of #2021 - fixing ignored integration tests
Problem
Multiple tests were ignored without tracking issues, leading to hidden bugs and technical debt accumulation. We need systematic prevention.
Root cause of parent issue:
- No enforcement preventing
#[ignore]without tracking - No visibility into accumulating ignored tests
- No documented policy for test management
- Pre-commit hooks don't catch ignored tests
Proposed Solution
Implement multi-layered defense to prevent future ignored test accumulation.
Layer 1: Pre-commit Hook Enhancement
Extend existing TODO-MUST-FIX pattern to cover ignored tests:
// TODO-MUST-FIX: This test has channel closed errors
#[tokio::test]
#[ignore = "Channel closed on startup - investigating"]
async fn test_foo() { ... }The pre-commit hook should block commits with TODO-MUST-FIX comments, forcing developer to:
- Fix the issue immediately, OR
- Create a tracking issue and link it
Layer 2: CI Check for Ignored Tests
Add CI job that fails if ignored tests exist without tracking issue references:
# Check for ignored tests without issue links
grep -r "#\[ignore" tests/ | grep -v "github.com/.*issues/[0-9]"Allowed patterns:
#[ignore = "github.com/freenet/freenet-core/issues/123"]#[ignore = "#123 - description"]#[ignore = "large scale test - run manually"](with justification)
Blocked patterns:
#[ignore](no reason)#[ignore = "fix me"](no tracking)#[ignore = "broken"](no issue link)
Layer 3: Documentation
Update CLAUDE.md and developer docs with clear test management policy:
## Test Management Policy
### Never Ignore Tests Without Tracking
If a test must be temporarily disabled:
1. Create a GitHub issue documenting:
- What's broken
- Why it's broken
- Steps to reproduce
- Investigation status
2. Link the issue in the ignore annotation:
```rust
#[ignore = "https://github.com/freenet/freenet-core/issues/123"]- OR fix the test immediately
Acceptable Ignore Reasons
Only these scenarios justify ignoring tests:
- Performance/Scale: "large scale test - run manually" (with docs)
- With tracking issue: Link to issue investigating the failure
- Temporary during development: Use TODO-MUST-FIX (blocks commit)
Never Acceptable
#[ignore]with no reason#[ignore = "broken"]or#[ignore = "fix me"]- Ignoring tests to make CI green
- "We'll fix it later" without an issue
### Layer 4: Regular Audits
Add to sprint planning / release checklist:
- Review ignored tests dashboard
- Verify tracking issues are still active
- Close old issues when tests are re-enabled
## Implementation Tasks
- [ ] Enhance pre-commit hook to check for TODO-MUST-FIX in test files
- [ ] Create CI job: `check-ignored-tests.sh` script
- [ ] Update `.github/workflows/ci.yml` to include ignored test check
- [ ] Document policy in `CLAUDE.md`
- [ ] Document policy in `CONTRIBUTING.md` (if it exists)
- [ ] Create ignored tests dashboard (GitHub project board or query)
- [ ] Add to pre-release checklist: "Review ignored tests"
## Example CI Check Script
```bash
#!/bin/bash
# check-ignored-tests.sh
set -e
echo "Checking for ignored tests without tracking issues..."
# Find all ignored tests
IGNORED_TESTS=$(find . -name "*.rs" -type f -exec grep -H "#\[ignore" {} \;)
if [ -z "$IGNORED_TESTS" ]; then
echo "✓ No ignored tests found"
exit 0
fi
# Check each ignored test has a tracking reference
VIOLATIONS=0
while IFS= read -r line; do
# Allow patterns with issue links or documented reasons
if echo "$line" | grep -qE "(github\.com|issues/[0-9]+|#[0-9]+|large scale|manual)"; then
continue
fi
# Allow empty ignores if followed by TODO-MUST-FIX comment above
# (pre-commit hook will catch these)
# Flag as violation
echo "❌ Ignored test without tracking: $line"
VIOLATIONS=$((VIOLATIONS + 1))
done <<< "$IGNORED_TESTS"
if [ $VIOLATIONS -gt 0 ]; then
echo ""
echo "Found $VIOLATIONS ignored test(s) without tracking issues"
echo "Please either:"
echo " 1. Fix the test and remove #[ignore]"
echo " 2. Create a tracking issue and reference it: #[ignore = \"#123 - reason\"]"
echo " 3. For manual-only tests, justify: #[ignore = \"large scale test - run manually\"]"
exit 1
fi
echo "✓ All ignored tests have proper tracking"
Success Criteria
- Pre-commit hook blocks TODO-MUST-FIX in tests
- CI check fails on untracked ignored tests
- Policy documented in CLAUDE.md
- Policy documented in CONTRIBUTING.md (if exists)
- Check script tested on current codebase
- All existing ignored tests brought into compliance (via parent issue)
- Dashboard/tracking mechanism established
Future Enhancements
- Auto-generate ignored tests report in CI
- Track "time ignored" metric
- Alert on tests ignored > 90 days
- Integration with GitHub Projects for tracking
Impact
Medium priority - prevents recurrence of parent issue. Should be implemented after fixing existing ignored tests to establish clean baseline.
[AI-assisted debugging and comment]
Metadata
Metadata
Assignees
Labels
Type
Projects
Status