Skip to content

Conversation

@bokelley
Copy link
Collaborator

Problem

  • Smoke tests had 18 deselected tests (requires_server, skip_ci markers)
  • 2 warnings: FastMCP deprecation, SQLAlchemy relationships
  • 280+ lines of obsolete SQLite tests (PostgreSQL-only architecture)
  • 279 lines of redundant server tests (duplicated integration tests)
  • Smoke tests required PostgreSQL in CI and took 7+ seconds

Solution

  1. Deleted all SQLite tests - 280 lines of tests for unsupported database
  2. Deleted test_smoke_critical_paths.py - 279 lines duplicating integration tests
  3. Fixed database-dependent test - test_config_loader_workstest_config_loader_imports (no DB needed)
  4. Fixed FastMCP warning - Removed deprecated stateless_http parameter
  5. Fixed SQLAlchemy warning - Added overlaps parameter to relationships
  6. Simplified CI workflow - Removed PostgreSQL service, migrations step, filter flags

Results

  • Before: 33 tests (15 selected, 18 deselected), 2 warnings, 1 skipped
  • After: 14 tests (all run), 0 warnings, 0 skipped
  • CI Impact: ~2 minutes faster (no PostgreSQL service)
  • Purpose: Fast import/sanity checks that catch catastrophic failures

Testing

# All smoke tests pass cleanly
uv run pytest tests/smoke/ -v
# 14 passed in 6.19s, 0 warnings, 0 skipped

Smoke tests now serve their actual purpose: fast, simple checks before running the full test suite.

**Problem:**
- Smoke tests had 18 deselected tests (requires_server, skip_ci)
- 2 warnings (FastMCP deprecation, SQLAlchemy relationships)
- 280+ lines of obsolete SQLite tests
- 279 lines of redundant server tests (duplicated integration tests)
- Smoke tests took 7s+ and required PostgreSQL in CI

**Solution:**
1. Deleted all SQLite migration tests (PostgreSQL-only architecture)
2. Deleted test_smoke_critical_paths.py (redundant with integration tests)
3. Fixed test_config_loader_works to not require database
4. Removed deprecated stateless_http parameter from FastMCP
5. Added overlaps parameter to SQLAlchemy relationships
6. Simplified CI workflow (no DB, no filters)

**Results:**
- Before: 33 tests (15 selected, 18 deselected), 2 warnings, 1 skipped
- After: 14 tests (all run), 0 warnings, 0 skipped
- CI: ~2 minutes faster (no PostgreSQL service needed)
- Purpose: Fast import/sanity checks before full test suite

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@bokelley bokelley merged commit 73cbc99 into main Oct 26, 2025
9 checks passed
bokelley added a commit that referenced this pull request Oct 26, 2025
Merged PRs #628 and #629 from main:
- fix: Integration tests, mypy errors, and deprecation warnings (#628)
- fix: Clean up smoke tests and resolve warnings (#629)

Conflict resolution:
- webhook_delivery_service.py: Accepted main's cleaner error handling
  (single try-except catching ValueError and OSError)

Changes from main:
- Schema regeneration (extra='forbid' → extra='ignore')
- Mypy configuration updates (1081 errors, down from 1586)
- Smoke test cleanup (removed test_smoke_critical_paths.py)
- Deprecation warning fixes
- Integration test improvements
bokelley added a commit that referenced this pull request Oct 26, 2025
Resolved conflicts in test_signals_agent_workflow.py:
- Updated to use origin/main's improved mocking patterns with ExitStack
- Removed signals_agent_config from tenant_dict (we removed that field)
- Used complete SignalDeployment fields (account, estimated_activation_duration_minutes)
- Renamed _add_test_products_v2 to _add_test_products
- Added add_required_setup_data call for proper test setup

Brings in:
- Integration test improvements (#631, #626, #629, #628)
- Mypy error reductions
- Test infrastructure improvements
- 100% integration_v2 test coverage
danf-newton pushed a commit to Newton-Research-Inc/salesagent that referenced this pull request Nov 24, 2025
**Problem:**
- Smoke tests had 18 deselected tests (requires_server, skip_ci)
- 2 warnings (FastMCP deprecation, SQLAlchemy relationships)
- 280+ lines of obsolete SQLite tests
- 279 lines of redundant server tests (duplicated integration tests)
- Smoke tests took 7s+ and required PostgreSQL in CI

**Solution:**
1. Deleted all SQLite migration tests (PostgreSQL-only architecture)
2. Deleted test_smoke_critical_paths.py (redundant with integration tests)
3. Fixed test_config_loader_works to not require database
4. Removed deprecated stateless_http parameter from FastMCP
5. Added overlaps parameter to SQLAlchemy relationships
6. Simplified CI workflow (no DB, no filters)

**Results:**
- Before: 33 tests (15 selected, 18 deselected), 2 warnings, 1 skipped
- After: 14 tests (all run), 0 warnings, 0 skipped
- CI: ~2 minutes faster (no PostgreSQL service needed)
- Purpose: Fast import/sanity checks before full test suite

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude <noreply@anthropic.com>
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