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
55 changes: 55 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
name: Test

on:
push:
branches: [main, master]
pull_request:
branches: [main, master]

env:
CARGO_TERM_COLOR: always
RUST_BACKTRACE: 1

jobs:
test:
runs-on: ubuntu-latest

steps:
- name: Checkout repository
uses: actions/checkout@v4

- name: Install Rust toolchain
uses: dtolnay/rust-action@stable
with:
components: clippy, rustfmt

- name: Cache Cargo registry
uses: actions/cache@v4
with:
path: |
~/.cargo/registry
~/.cargo/git
key: ${{ runner.os }}-cargo-registry-${{ hashFiles('**/Cargo.lock') }}
restore-keys: |
${{ runner.os }}-cargo-registry-

- name: Cache Cargo target directory
uses: actions/cache@v4
with:
path: target
key: ${{ runner.os }}-cargo-target-${{ hashFiles('**/Cargo.lock') }}-${{ github.sha }}
restore-keys: |
${{ runner.os }}-cargo-target-${{ hashFiles('**/Cargo.lock') }}-
${{ runner.os }}-cargo-target-

- name: Check formatting
run: cargo fmt --all -- --check

- name: Run clippy lint
run: cargo clippy --all-targets --all-features -- -D warnings

- name: Build project
run: cargo build --release

- name: Run tests
run: cargo test --all-features --verbose
12 changes: 6 additions & 6 deletions .sisyphus/boulder.json
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
{
"active_plan": "/Users/xfy/Developer/aether/.sisyphus/plans/documentation-improvement.md",
"started_at": "2026-03-09T16:00:12.935Z",
"active_plan": "/Users/xfy/Developer/aether-unit-tests/.sisyphus/plans/test-improvement.md",
"started_at": "2026-03-10T08:05:00.000Z",
"session_ids": [
"ses_32de363ebffe3QGuZ0LbYy3eZR"
"ses_329469bfaffeWuJ8M7277NkmZb"
],
"plan_name": "documentation-improvement",
"plan_name": "test-improvement",
"agent": "atlas",
"worktree_path": "/Users/xfy/Developer/aether-worktrees/docs-improvement"
}
"worktree_path": "/Users/xfy/Developer/aether-unit-tests"
}
33 changes: 33 additions & 0 deletions .sisyphus/notepads/learnings.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,36 @@

**Output**: `target/doc/aether_matrix/index.html` (7.2KB)


## Test Coverage Tool Setup (Tue Mar 10 2026)

**Goal**: Establish test coverage reporting capability

**Platform Limitations**:
- `cargo-tarpaulin` requires Linux (not available on macOS)
- `cargo-llvm-cov` is cross-platform alternative

**Solution Created**:
1. Shell script: `scripts/coverage.sh` - auto-detects available tool
2. Makefile target: `make coverage` - integrates with build system
3. Issue documented: `.sisyphus/notepads/test-improvement/issues.md`

**Usage**:
```bash
make coverage # Run via Makefile
./scripts/coverage.sh # Run via shell script
./scripts/coverage.sh html # HTML report only
```

**Installation Options**:
```bash
# Linux (recommended)
cargo install cargo-tarpaulin

# macOS / Cross-platform
cargo install cargo-llvm-cov
```

**Files Created**:
- `scripts/coverage.sh` (executable)
- `Makefile` (updated with coverage target)
70 changes: 70 additions & 0 deletions .sisyphus/notepads/test-improvement/issues.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
# Chat History Functionality Issues

## Issue: Missing Chat History Implementation

**Date**: Tue Mar 10 2026

**Description**:
The database schema includes a `chat_history` table (defined in `migrations/20260305000000_init.sql`), but there is no actual implementation that uses this table for persistent chat history storage.

**Current State**:
- ✅ `chat_history` table exists in database schema
- ✅ Table has proper structure with indexes (`room_id`, `created_at`)
- ❌ No methods in `Database` struct to interact with `chat_history`
- ❌ Conversation history is managed entirely in memory via `ConversationManager`
- ❌ No integration between conversation management and database persistence

**Evidence**:
- `src/store/database.rs` contains only connection and migration logic, no chat history methods
- `src/conversation.rs` manages all conversation state in memory using `HashMap<String, Vec<ChatCompletionRequestMessage>>`
- `src/ai_service.rs` uses only the in-memory `ConversationManager`
- No SQL queries found that interact with `chat_history` table
- Grep search confirms no usage of `chat_history` outside of schema definition and test verification

**Impact**:
- Chat history is lost when the bot restarts
- No persistent conversation history across sessions
- Database table is unused (wasted storage)

**Recommendation**:
Implement chat history persistence by:
1. Adding methods to `Database` struct for saving/loading chat history
2. Integrating database persistence into `ConversationManager`
3. Creating comprehensive tests for chat history functionality
---

## Issue: cargo-tarpaulin Not Available for Test Coverage

**Date**: Tue Mar 10 2026

**Description**:
`cargo-tarpaulin` (Rust code coverage tool) is not installed and cannot be used to generate test coverage reports.

**Current State**:
- ❌ `cargo tarpaulin` command not found
- ❌ No alternative coverage tool configured
- ❌ No coverage reporting scripts available
- ❌ No `.codecov.yml` or similar configuration

**Impact**:
- Cannot measure test coverage percentage
- Cannot identify untested code paths
- Cannot track coverage trends over time
- No visibility into testing gaps

**Attempted Resolution**:
```bash
cargo tarpaulin --version
# Result: error: no such command: `tarpaulin`
```

**Recommended Actions**:
1. Install cargo-tarpaulin: `cargo install cargo-tarpaulin`
2. Create coverage script: `scripts/coverage.sh`
3. Add coverage configuration to project root
4. Consider CI integration (GitHub Actions, Codecov)

**Platform Note**:
cargo-tarpaulin requires Linux and may not work on macOS. Alternative tools:
- `cargo-llvm-cov` (cross-platform, LLVM-based)
- `tarpaulin` in Docker/Linux CI only
96 changes: 96 additions & 0 deletions .sisyphus/notepads/test-improvement/learnings.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
# Test Infrastructure Improvements - Learnings

## Date: 2026-03-10

### Key Learnings

1. **Test Utility Module Structure**
- `tests/common/mod.rs` - Central module exports
- `tests/common/test_helpers.rs` - Streaming test helpers
- `tests/common/test_utils.rs` - General test utilities (logging, temp dirs)
- Existing modules: `mock_client.rs`, `mock_room.rs`

2. **Common Compilation Errors Fixed**
- `AiShip::new` -> `AiService::new` (typo in test file)
- `McpConfig` import path: `aether_matrix::mcp::McpConfig` (not from `config` module)
- `ToolRegistry` uses `is_empty()` instead of `len()`
- `AiServiceTrait` must be imported to use `has_tools()` method

3. **Trait Implementation Requirements**
- Mock implementations must implement ALL trait methods
- Missing methods added to mocks:
- `fn inner_mcp_registry(&self) -> Option<Arc<RwLock<ToolRegistry>>>`
- `async fn has_tools(&self) -> bool`

4. **Test Logging Setup**
- Use `tracing_subscriber::EnvFilter` for configurable log levels
- Use `Once` to ensure single initialization per test process
- Use `.with_test_writer()` for proper test output capture

5. **Best Practices**
- Keep test utilities in `tests/common/` for sharing across test files
- Always implement full trait methods in mocks, even if just returning defaults
- Use `cargo check --tests` for fast compilation verification before running tests

## Database Testing Learnings

### Key Insights:
1. **Integration Test Structure**: Rust integration tests in the `tests/` directory need to be structured as separate files with `#[tokio::test]` for async functionality.

2. **Common Utilities**: When using shared test utilities, it's sometimes easier to copy them directly into the test file rather than dealing with complex module imports that can break due to dependencies on other broken modules.

3. **Error Handling Variability**: Error messages can vary significantly across different operating systems and environments. Tests should be flexible in their error message assertions to handle this variability.

4. **Database Path Edge Cases**:
- Empty paths may create in-memory databases instead of failing
- Invalid character paths (like null bytes) are better test cases for path validation
- Permission-denied scenarios should use system-specific protected paths like `/root/`

5. **SQLite Behavior**:
- SQLite handles empty strings differently than expected (creates in-memory DB)
- Foreign key constraints need to be explicitly enabled
- Migration scripts should use `CREATE TABLE IF NOT EXISTS` for idempotency

6. **Concurrency Testing**:
- Use `Arc<Mutex<>>` pattern for sharing database connections across threads
- Thread spawning works well for testing concurrent access patterns
- Each thread should operate independently to avoid race conditions in tests

7. **Test Organization**:
- Comprehensive test suites should cover connection, structure, concurrency, error handling, and migration scenarios
- Each test should have a clear, descriptive name indicating what it verifies
- Inline comments explaining test intent improve maintainability

### Best Practices Established:
- Use temporary directories for database tests to avoid file conflicts
- Test both single-threaded and multi-threaded access patterns
- Verify foreign key constraints are properly enabled
- Test migration idempotency by creating multiple database instances pointing to the same file
- Include comprehensive error handling tests for various invalid path scenarios

## PersonaStore Test Suite Learnings

### Key Insights
1. **Database Constraints**: SQLite allows empty strings in PRIMARY KEY and NOT NULL columns, which means validation must be handled at the application level if needed.

2. **Test Structure**: The existing test suite uses a `create_test_store()` helper function that creates a temporary database with migrations applied, ensuring proper isolation.

3. **Error Handling**: The current implementation doesn't validate for empty strings, so tests should reflect actual behavior rather than expected domain constraints.

4. **Boundary Cases**: Long text fields (10KB+) work fine with SQLite, and Unicode characters are properly handled throughout the stack.

5. **Room Operations**: Room persona associations can be updated multiple times, and operations on non-existent rooms/IDs behave as expected.

### Test Coverage Added
- **Duplicate ID handling**: Verified that creating personas with duplicate IDs fails due to PRIMARY KEY constraint
- **Empty value handling**: Confirmed that empty strings are stored successfully (reflecting actual DB behavior)
- **Boundary conditions**: Tested very long prompts (10KB) and special Unicode characters
- **Room operations**: Comprehensive coverage of edge cases for room-persona associations
- **Validation scenarios**: Verified avatar emoji handling with None, empty string, and Unicode values
- **Sorting behavior**: Confirmed that `get_all()` returns builtin personas first, then custom ones sorted by name

### Testing Best Practices Applied
- Each test uses isolated temporary databases via `tempfile::TempDir`
- Test names are descriptive and self-explanatory (no inline comments needed)
- All tests verify both success and failure scenarios appropriately
- Used realistic test data that matches the actual domain usage patterns
Loading
Loading