Skip to content

Commit e5a861c

Browse files
chaliyclaude
andauthored
fix(test): isolate fail-point tests for CI execution (#26)
## Summary - Remove `#[ignore]` attributes from all 14 fail-point tests - Split CI test step: regular tests with `--features network`, then fail-point tests with `--features failpoints --test-threads=1` - Add dedicated `test-failpoints` recipe in justfile The fail-point tests use global state from `fail-rs` and must run single-threaded. They are now isolated by feature flag and run as a separate CI step. ## Test plan - [x] Verify all 14 fail-point tests pass with `--test-threads=1` - [x] Verify regular tests still pass with `--features network` - [x] Verify `cargo fmt --check` and `cargo clippy` pass - [x] Ran tests 5+ times to confirm no flakiness Co-authored-by: Claude <noreply@anthropic.com>
1 parent cfd7ef4 commit e5a861c

3 files changed

Lines changed: 14 additions & 23 deletions

File tree

.github/workflows/ci.yml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,10 @@ jobs:
4343
run: cargo clippy --all-targets --all-features -- -D warnings
4444

4545
- name: Run tests
46-
run: cargo test --all-features
46+
run: cargo test --features network
47+
48+
- name: Run fail-point tests (single-threaded)
49+
run: cargo test --features failpoints --test security_failpoint_tests -- --test-threads=1
4750

4851
- name: Run examples
4952
run: |

crates/bashkit/tests/security_failpoint_tests.rs

Lines changed: 3 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,9 @@
55
//! - Filesystem operations fail gracefully
66
//! - Interpreter handles errors without exposing internal state
77
//!
8-
//! **IMPORTANT**: Fail points are global state and not thread-safe.
9-
//! Run these tests with a single thread to avoid race conditions:
10-
//!
11-
//! ```sh
12-
//! cargo test --features failpoints security_ -- --test-threads=1
13-
//! ```
8+
//! **NOTE**: These tests use global state and must run single-threaded.
9+
//! CI runs them separately with `--test-threads=1`. The tests are isolated
10+
//! from the main test suite by using the `failpoints` feature flag.
1411
1512
#![cfg(feature = "failpoints")]
1613

@@ -48,7 +45,6 @@ async fn run_script_with_limits(script: &str, limits: ExecutionLimits) -> ExecRe
4845
/// Security property: Even if the counter is corrupted to skip increment,
4946
/// the limit should still be enforced eventually.
5047
#[tokio::test]
51-
#[ignore = "Fail point test requires --test-threads=1"]
5248
async fn security_command_limit_skip_increment() {
5349
fail::cfg("limits::tick_command", "return(skip_increment)").unwrap();
5450

@@ -69,7 +65,6 @@ async fn security_command_limit_skip_increment() {
6965

7066
/// Test: Command counter overflow is handled
7167
#[tokio::test]
72-
#[ignore = "Fail point test requires --test-threads=1"]
7368
async fn security_command_limit_overflow() {
7469
fail::cfg("limits::tick_command", "return(force_overflow)").unwrap();
7570

@@ -87,7 +82,6 @@ async fn security_command_limit_overflow() {
8782

8883
/// Test: Loop counter reset doesn't cause infinite loop
8984
#[tokio::test]
90-
#[ignore = "Fail point test requires --test-threads=1 and is flaky in parallel"]
9185
async fn security_loop_counter_reset() {
9286
// Note: This test would cause infinite loop if limit wasn't also checked elsewhere
9387
// We set a reasonable iteration limit to prevent actual infinite loop
@@ -111,7 +105,6 @@ async fn security_loop_counter_reset() {
111105

112106
/// Test: Function depth bypass is detected
113107
#[tokio::test]
114-
#[ignore = "Fail point test requires --test-threads=1"]
115108
async fn security_function_depth_bypass() {
116109
fail::cfg("limits::push_function", "return(skip_check)").unwrap();
117110

@@ -148,7 +141,6 @@ async fn security_function_depth_bypass() {
148141

149142
/// Test: Read failure is handled gracefully
150143
#[tokio::test]
151-
#[ignore = "Fail point test requires --test-threads=1"]
152144
async fn security_fs_read_io_error() {
153145
fail::cfg("fs::read_file", "return(io_error)").unwrap();
154146

@@ -162,7 +154,6 @@ async fn security_fs_read_io_error() {
162154

163155
/// Test: Permission denied is handled
164156
#[tokio::test]
165-
#[ignore = "Fail point test requires --test-threads=1"]
166157
async fn security_fs_read_permission_denied() {
167158
fail::cfg("fs::read_file", "return(permission_denied)").unwrap();
168159

@@ -183,7 +174,6 @@ async fn security_fs_read_permission_denied() {
183174

184175
/// Test: Corrupt data doesn't cause crash
185176
#[tokio::test]
186-
#[ignore = "Fail point test requires --test-threads=1"]
187177
async fn security_fs_corrupt_data() {
188178
fail::cfg("fs::read_file", "return(corrupt_data)").unwrap();
189179

@@ -199,7 +189,6 @@ async fn security_fs_corrupt_data() {
199189

200190
/// Test: Write failure doesn't corrupt state
201191
#[tokio::test]
202-
#[ignore = "Fail point test requires --test-threads=1"]
203192
async fn security_fs_write_failure() {
204193
fail::cfg("fs::write_file", "return(io_error)").unwrap();
205194

@@ -213,7 +202,6 @@ async fn security_fs_write_failure() {
213202

214203
/// Test: Disk full is handled
215204
#[tokio::test]
216-
#[ignore = "Fail point test requires --test-threads=1"]
217205
async fn security_fs_disk_full() {
218206
fail::cfg("fs::write_file", "return(disk_full)").unwrap();
219207

@@ -231,7 +219,6 @@ async fn security_fs_disk_full() {
231219

232220
/// Test: Command execution error is handled
233221
#[tokio::test]
234-
#[ignore = "Fail point test requires --test-threads=1"]
235222
async fn security_interp_execution_error() {
236223
fail::cfg("interp::execute_command", "return(error)").unwrap();
237224

@@ -245,7 +232,6 @@ async fn security_interp_execution_error() {
245232

246233
/// Test: Non-zero exit code injection
247234
#[tokio::test]
248-
#[ignore = "Fail point test requires --test-threads=1"]
249235
async fn security_interp_exit_nonzero() {
250236
fail::cfg("interp::execute_command", "return(exit_nonzero)").unwrap();
251237

@@ -264,7 +250,6 @@ async fn security_interp_exit_nonzero() {
264250

265251
/// Test: Multiple fail points active simultaneously
266252
#[tokio::test]
267-
#[ignore = "Fail point test requires --test-threads=1"]
268253
async fn security_multiple_failpoints() {
269254
// Activate multiple fail points
270255
fail::cfg("limits::tick_command", "5%return(skip_increment)").unwrap();
@@ -292,7 +277,6 @@ async fn security_multiple_failpoints() {
292277

293278
/// Test: Fail point with probability (fuzz-like testing)
294279
#[tokio::test]
295-
#[ignore = "Fail point test requires --test-threads=1"]
296280
async fn security_probabilistic_failures() {
297281
// 10% chance of failure on each command
298282
fail::cfg("limits::tick_command", "10%return(corrupt_high)").unwrap();
@@ -330,7 +314,6 @@ async fn security_probabilistic_failures() {
330314

331315
/// Demonstrates how to use fail points for custom security testing
332316
#[tokio::test]
333-
#[ignore = "Fail point test requires --test-threads=1"]
334317
async fn security_example_custom_failpoint_usage() {
335318
// Setup: Configure fail point
336319
fail::cfg("fs::write_file", "return(permission_denied)").unwrap();

justfile

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,14 @@ default:
1212
build:
1313
cargo build
1414

15-
# Run all tests
15+
# Run all tests (including fail-point tests)
1616
test:
17-
cargo test --all-features
17+
cargo test --features network
18+
cargo test --features failpoints --test security_failpoint_tests -- --test-threads=1
19+
20+
# Run fail-point tests only (single-threaded, requires failpoints feature)
21+
test-failpoints:
22+
cargo test --features failpoints --test security_failpoint_tests -- --test-threads=1
1823

1924
# Run formatters and linters (auto-fix)
2025
fmt:

0 commit comments

Comments
 (0)