Skip to content

Fix test flakiness - #116

Open
radubahmata wants to merge 6 commits into
protocol-security:masterfrom
radubahmata:test/fix-flaky-tests
Open

Fix test flakiness#116
radubahmata wants to merge 6 commits into
protocol-security:masterfrom
radubahmata:test/fix-flaky-tests

Conversation

@radubahmata

@radubahmata radubahmata commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Isolate tests from host Git configuration.
  • Replace fixed delays and wall-clock assertions with observable state and behavior.
  • Extend ShellCheck coverage to every test script.

Changes

  • .github/workflows/ci.yml: lint all scripts under tests/.
  • tests/_test_env.sh: provide shared Git configuration isolation.
  • tests/test_ci_workflows.sh: verify all test scripts load the shared environment and receive CI lint coverage.
  • tests/test_harness.sh: use a fixed hlog timestamp and explicit writable Git configuration for signing tests.
  • tests/runtime_signal_trap.sh: wait for the expected commit before sending signals.
  • tests/test_drivers.sh: bound reaper and watchdog tests without asserting wall-clock duration.
  • tests/test_launch.sh and tests/test_session_end_push.sh: replace local Git isolation with the shared environment.
  • Remaining test scripts: load the shared test environment to prevent host Git settings from affecting results.

Self-review checklist

  • This PR addresses a single concern (why?). If it covers multiple independent changes, I've split them into separate PRs.

Test plan

  • Run ./tests/test_drivers.sh.
  • Run ./tests/test.sh --all.
  • Run ShellCheck at warning severity on all shell scripts.

Comment thread tests/test_harness.sh

assert_contains "hlog timestamp" \
"$(date +%H:%M:%S)" "$PLAIN"
"12:34:56 harness[3] test message" "$PLAIN"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, was that adding to the flakiness?

@radubahmata radubahmata Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, if it gets to the next second between the two date invocations. previously it compared the real-time string to one extracted a few milliseconds before.

@radubahmata radubahmata changed the title [DRAFT] Fix test flakiness Fix test flakiness Sep 2, 2026
@radubahmata
radubahmata marked this pull request as ready for review September 2, 2026 22:05
@radubahmata

Copy link
Copy Markdown
Contributor Author

Note: usually tests are append-only, but the decision to change the ones covered by this PR is backed by failures encountered in other CI/local test runs. The most recent example: https://github.com/protocol-security/claude-swarm/actions/runs/33561114313/job/100033552849.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants