Skip to content

fix(#936): refresh stale test expectations for #665, #898, #474 - #940

Merged
vybe merged 1 commit into
devfrom
fix/936-stale-test-expectations
May 25, 2026
Merged

fix(#936): refresh stale test expectations for #665, #898, #474#940
vybe merged 1 commit into
devfrom
fix/936-stale-test-expectations

Conversation

@obasilakis

Copy link
Copy Markdown
Contributor

Closes #936.

Summary

  • test_get_timeout_default_is_900test_get_timeout_default_is_3600 (default raised in feat: increase default chat execution timeout from 15m to 60m #665).
  • TestCheckNetworkHealthClassification: rename test_{read_timeout,read_error,pool_timeout}_does_not_record..._records_failure and flip the assertion to == 1. /health probes treat these exceptions as agent-liveness signals and DO record on the circuit (override of TRANSIENT_TRANSPORT_EXCEPTIONS).
  • TestCreateFolder: add a small _wait_for_container_ready helper that polls until the agent container is visible to the backend's Docker SDK, and call it from the two tests that actually hit POST /files/mkdir (feat(files): create folder in File Manager (#37) #898). The other three tests in the class fail-fast before the container lookup and don't need the wait.

No production code changes — tests only.

Test plan

Note: tests/integration/test_monitoring_service.py requires REDIS_URL to be exported before pytest starts (the module sets localhost:6379 at import time, but tests/conftest.py pre-loads services.agent_client first, which captures config.REDIS_URL). Pre-existing setup quirk, not introduced here.

Several integration tests were asserting pre-change behaviour:

- tests/test_agent_timeout.py — default execution_timeout_seconds was
  raised from 900 → 3600 in #665. Rename
  test_get_timeout_default_is_900 and update the expected value.

- tests/integration/test_monitoring_service.py — /health-specific
  override in monitoring_service.py treats ReadTimeout, ReadError, and
  PoolTimeout as agent liveness signals and DOES record_failure() on
  the per-agent circuit. Rename the three `_does_not_record_` tests
  to `_records_failure` and flip the assertion from 0 → 1. The
  docstrings now point at the explicit /health-specific handlers
  rather than the unrelated TRANSIENT_TRANSPORT_EXCEPTIONS contract.

- tests/test_agent_files.py — POST /api/agents/{name}/files/mkdir
  (#898) is the first test in the file that requires the agent
  container to be visible to the backend Docker SDK before the
  request, not just the row marked status=running. Add a small
  _wait_for_container_ready helper that polls /files until the
  container is visible (probe returns anything other than 404) and
  call it in the two TestCreateFolder tests that actually hit the
  endpoint (test_create_folder_and_list, test_create_folder_duplicate_returns_409).
  The other three tests in the class fail-fast before the container
  check (deny-list 403, nonexistent-agent 404, unauth 401) and don't
  need the wait.

No production code changes — tests only.

@vybe vybe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved per /validate-pr — pure test maintenance realigning assertions with shipped behavior in #665, #898, #474. Clean security scan.

@vybe
vybe merged commit aa75908 into dev May 25, 2026
12 checks passed
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