feat: split long running UTs

[arnaubennassar]

🔄 Changes Summary

• Split unit tests that are "pure" vs the ones that have deps that make them slow

⚠️ Breaking Changes

None

📋 Config Updates

None

✅ Testing

tested that things keep working... UTs still take 20s to run

🐞 Issues

• Closes [FEATURE] Split unittest between short and long #321

[claude]

Claude finished @arnaubennassar's task in 1m 17s —— View job

---

Review Complete ✅

I've reviewed this PR that splits unit tests into fast and slow categories. The implementation is clean and follows Go testing best practices.

---

🚀 What's New

• Added new test-unit-short target in Makefile that skips slow integration tests using the -short flag
• Added testing.Short() guards to 22 integration tests across 13 files to enable fast test runs
• Tests are now properly categorized: fast unit tests vs slower integration tests that use simulated backends, Docker, or external dependencies

🐛 Bug Fixes

None.

📋 Config Updates

None.

⚠️ Breaking Changes

None.

---

Review Feedback

✅ Strengths

1. Consistent Implementation: All 22 tests follow the exact same pattern with proper skip messages

2. Clean Makefile Addition: The new test-unit-short target is well-documented and follows existing conventions

3. Correct Test Selection: The tests marked for skipping are truly integration tests that:

   • Use simulated.Backend for blockchain simulation
   • Require Docker/Geth containers
   • Have network dependencies or RPC calls
   • Take significantly longer to run

4. Backward Compatibility: The existing make test-unit command remains unchanged, so CI/CD pipelines continue to work

🟢 Observations

Test Coverage Breakdown:

• aggoracle: 2 E2E tests
• bridgesync: 5 E2E/integration tests
• claimsync: 4 simulated backend tests
• etherman: 1 upgrade test
• l1infotreesync: 3 E2E tests
• l2gersync: 3 E2E tests with legacy contract support
• pprof: 1 HTTP server test
• reorgdetector: 6 integration tests

Performance Impact: Based on the PR description mentioning "UTs still take 20s to run," the fast tests complete quickly while the full suite with integration tests remains comprehensive.

🟡 Suggestions (Optional)

1. Documentation Enhancement: Consider adding a comment in the Makefile explaining when to use test-unit-short vs test-unit:

   # Use test-unit-short for rapid feedback during development
   # Use test-unit for comprehensive testing before commits

2. CI Integration: You might want to add a fast CI job that runs make test-unit-short on every commit for quick feedback, keeping the full test suite for PR validation.

3. Editor Integration Hint: The Makefile comment mentions "suitable for editor on-save runs" which is excellent - consider adding this to the development docs.

✅ Code Quality

• Consistency: Perfect - all tests use identical guard pattern
• Test Semantics: Correct - testing.Short() is the idiomatic Go approach for this
• Skip Messages: Clear and descriptive
• No Logic Changes: The tests themselves remain untouched, only guards added

Conclusion

This is a well-executed PR that improves developer experience by enabling fast test iterations. The implementation is clean, consistent, and follows Go best practices. The categorization of tests is accurate, and the approach is maintainable.

Recommendation: ✅ Approve

---

[joanestebanr]

I suggest that CI also runs the short test individually for this reason:

• If unit-test fails will be so easy to known if are the long or short ones
• To prevent a non-short test from being mistakenly classified as short.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[joanestebanr]

LGTM, I would suggest to add a Makefile target "test-unit-all" to run the all tests, shorts and logs