Skip to content
Merged
Show file tree
Hide file tree
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
70 changes: 70 additions & 0 deletions ALERTING_TESTS_PANIC_FIX.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
# Alerting Integration Tests - Panic-Prone Calls Fix

## Summary
Fixed all 15 panic-prone `unwrap()` calls in `tests/alerting_integration.rs` by replacing them with `expect()` calls that include descriptive error messages explaining the failure context.

## Changes Made

### File: `tests/alerting_integration.rs`

#### Metric Registration Functions (13 instances)
Replaced `unwrap()` with `expect()` including context-specific error messages in:
- `make_http_requests_total()` - Line 34
- `make_cngn_transactions_total()` - Line 44
- `make_stellar_submissions_total()` - Line 54
- `make_worker_errors_total()` - Line 64
- `make_worker_cycles_total()` - Line 74
- `make_db_errors_total()` - Line 84
- `make_payment_provider_failures_total()` - Line 94
- `make_exchange_rate_last_updated()` - Line 104
- `make_worker_last_cycle_timestamp()` - Line 114
- `make_pending_transactions_stale()` - Line 124
- `make_rate_limit_breaches_total()` - Line 134
- `make_cache_hits_total()` - Line 144
- `make_cache_misses_total()` - Line 154

**Error message pattern**: "Failed to register {metric_name} metric - this is a test setup error indicating registry conflict"

#### Render Function (2 instances)
Replaced `unwrap()` with `expect()` in the `render()` function:
1. **Encoder encoding** - Line 162: "Failed to encode Prometheus metrics - this indicates a serialization error in the test"
2. **UTF-8 conversion** - Line 164: "Failed to convert Prometheus metrics to UTF-8 - this indicates corrupt metric data in the test"

## Rationale

### Why `expect()` instead of `Result` propagation?
These are test helper functions that establish invariants required for tests to run. Failures here indicate:
- Registry conflicts (metric already registered)
- Serialization errors (corrupt internal state)
- UTF-8 conversion errors (corrupt metric data)

All of these are **unrecoverable test setup errors** that should halt execution immediately with clear diagnostic information.

### Documented Invariants
Each `expect()` call includes a descriptive message that:
1. Identifies what failed
2. Explains why it failed (root cause category)
3. Helps developers diagnose the issue quickly

## Acceptance Criteria ✓

- [x] All 15 avoidable `unwrap()` calls removed
- [x] Each `expect()` includes justified, descriptive error context
- [x] Error messages preserve observability context
- [x] No diagnostics errors found in the file
- [x] Changes maintain test isolation (each test uses independent registries)

## Testing

File passes static analysis with no diagnostics errors. The use of `expect()` is justified because:
1. These are test-only helpers, not production code
2. Failures indicate test setup issues, not runtime errors
3. Each panic is well-documented with clear error messages
4. Tests cannot meaningfully continue if metric registration fails

## Notes

- No production code affected (test file only)
- Test structure unchanged - still using isolated registries
- All error messages follow consistent format
- Zero remaining `unwrap()` calls in the file
33 changes: 18 additions & 15 deletions tests/alerting_integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ mod alerting_metrics_tests {
&["method", "route", "status_code"],
r
)
.unwrap()
.expect("Failed to register aframp_http_requests_total metric - this is a test setup error indicating registry conflict")
}

fn make_cngn_transactions_total(r: &Registry) -> prometheus::CounterVec {
Expand All @@ -41,7 +41,7 @@ mod alerting_metrics_tests {
&["tx_type", "status"],
r
)
.unwrap()
.expect("Failed to register aframp_cngn_transactions_total metric - this is a test setup error indicating registry conflict")
}

fn make_stellar_submissions_total(r: &Registry) -> prometheus::CounterVec {
Expand All @@ -51,7 +51,7 @@ mod alerting_metrics_tests {
&["status"],
r
)
.unwrap()
.expect("Failed to register aframp_stellar_tx_submissions_total metric - this is a test setup error indicating registry conflict")
}

fn make_worker_errors_total(r: &Registry) -> prometheus::CounterVec {
Expand All @@ -61,7 +61,7 @@ mod alerting_metrics_tests {
&["worker", "error_type"],
r
)
.unwrap()
.expect("Failed to register aframp_worker_errors_total metric - this is a test setup error indicating registry conflict")
}

fn make_worker_cycles_total(r: &Registry) -> prometheus::CounterVec {
Expand All @@ -71,7 +71,7 @@ mod alerting_metrics_tests {
&["worker"],
r
)
.unwrap()
.expect("Failed to register aframp_worker_cycles_total metric - this is a test setup error indicating registry conflict")
}

fn make_db_errors_total(r: &Registry) -> prometheus::CounterVec {
Expand All @@ -81,7 +81,7 @@ mod alerting_metrics_tests {
&["error_type"],
r
)
.unwrap()
.expect("Failed to register aframp_db_errors_total metric - this is a test setup error indicating registry conflict")
}

fn make_payment_provider_failures_total(r: &Registry) -> prometheus::CounterVec {
Expand All @@ -91,7 +91,7 @@ mod alerting_metrics_tests {
&["provider", "failure_reason"],
r
)
.unwrap()
.expect("Failed to register aframp_payment_provider_failures_total metric - this is a test setup error indicating registry conflict")
}

fn make_exchange_rate_last_updated(r: &Registry) -> prometheus::GaugeVec {
Expand All @@ -101,7 +101,7 @@ mod alerting_metrics_tests {
&["currency_pair"],
r
)
.unwrap()
.expect("Failed to register aframp_exchange_rate_last_updated_timestamp_seconds metric - this is a test setup error indicating registry conflict")
}

fn make_worker_last_cycle_timestamp(r: &Registry) -> prometheus::GaugeVec {
Expand All @@ -111,7 +111,7 @@ mod alerting_metrics_tests {
&["worker"],
r
)
.unwrap()
.expect("Failed to register aframp_worker_last_cycle_timestamp_seconds metric - this is a test setup error indicating registry conflict")
}

fn make_pending_transactions_stale(r: &Registry) -> prometheus::GaugeVec {
Expand All @@ -121,7 +121,7 @@ mod alerting_metrics_tests {
&["tx_type"],
r
)
.unwrap()
.expect("Failed to register aframp_pending_transactions_stale_total metric - this is a test setup error indicating registry conflict")
}

fn make_rate_limit_breaches_total(r: &Registry) -> prometheus::CounterVec {
Expand All @@ -131,7 +131,7 @@ mod alerting_metrics_tests {
&["endpoint"],
r
)
.unwrap()
.expect("Failed to register aframp_rate_limit_breaches_total metric - this is a test setup error indicating registry conflict")
}

fn make_cache_hits_total(r: &Registry) -> prometheus::CounterVec {
Expand All @@ -141,7 +141,7 @@ mod alerting_metrics_tests {
&["key_prefix"],
r
)
.unwrap()
.expect("Failed to register aframp_cache_hits_total metric - this is a test setup error indicating registry conflict")
}

fn make_cache_misses_total(r: &Registry) -> prometheus::CounterVec {
Expand All @@ -151,14 +151,17 @@ mod alerting_metrics_tests {
&["key_prefix"],
r
)
.unwrap()
.expect("Failed to register aframp_cache_misses_total metric - this is a test setup error indicating registry conflict")
}

fn render(r: &Registry) -> String {
let encoder = TextEncoder::new();
let mut buf = Vec::new();
encoder.encode(&r.gather(), &mut buf).unwrap();
String::from_utf8(buf).unwrap()
encoder
.encode(&r.gather(), &mut buf)
.expect("Failed to encode Prometheus metrics - this indicates a serialization error in the test");
String::from_utf8(buf)
.expect("Failed to convert Prometheus metrics to UTF-8 - this indicates corrupt metric data in the test")
}

// -----------------------------------------------------------------------
Expand Down
Loading