Skip to content

fix(tests): update test expectations to match upstream code changes - #18895

Closed
liuhao1024 wants to merge 4 commits into
NousResearch:mainfrom
liuhao1024:fix/ci-test-expectations
Closed

fix(tests): update test expectations to match upstream code changes#18895
liuhao1024 wants to merge 4 commits into
NousResearch:mainfrom
liuhao1024:fix/ci-test-expectations

Conversation

@liuhao1024

@liuhao1024 liuhao1024 commented May 2, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fix 10 CI test failures caused by upstream code changes that outpaced their test expectations.

Related Issue

N/A

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

tests/acp/test_server.py — add steer and queue to expected available commands list (added in recent TUI updates).

tests/gateway/test_teams.py — add microsoft_teams.common.http.client mock modules to sys.modules so the Teams adapter import chain resolves.

tests/hermes_cli/test_gateway_service.py — update TimeoutStopSec assertion from 90 to 210 to match the systemd unit template change.

tests/hermes_cli/test_update_gateway_restart.py — replace static find_gateway_pids return values with counter-based side effects. The upstream code now calls find_gateway_pids twice (initial kill + survivor sweep); the counter ensures the survivor sweep returns an empty list so SIGKILL doesn't double-hit already-terminated PIDs.

tests/run_agent/test_concurrent_interrupt.py — add _tool_guardrails and _append_guardrail_observation stubs to _Stub class; update slow_tool and polling_tool signatures to accept messages and **kwargs (new parameters added to _invoke_tool).

tests/tools/test_dockerfile_pid1_reaping.py — adapt assertions to the new Dockerfile structure (COPY directory + RUN cp instead of individual COPY lines).

How to Test

  1. Run pytest tests/ -q — all tests should pass
  2. Verify the specific scenario described above is resolved

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 26.4.1

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture and workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A

liuhao1024 added 2 commits May 3, 2026 00:37
- test_server.py: add steer/queue to expected commands list
- test_gateway_service.py: update TimeoutStopSec from 90 to 210
- test_update_gateway_restart.py: add counter-based find_gateway_pids to handle survivor sweep
- test_concurrent_interrupt.py: add _tool_guardrails, _append_guardrail_observation, update function signatures
- test_dockerfile_pid1_reaping.py: adapt to new Dockerfile COPY structure
- test_teams.py: add microsoft_teams.common.http/client mocks to sys.modules

These changes align test expectations with upstream code changes in run_agent.py, hermes_cli/main.py, and the new Dockerfile structure.
Change _find_call_count threshold from <= 2 to <= 1 so that the
survivor sweep (second find_gateway_pids call) returns an empty list,
preventing SIGKILL of PIDs already killed via SIGTERM.
@alt-glitch alt-glitch added type/test Test coverage or test infrastructure P3 Low — cosmetic, nice to have comp/gateway Gateway runner, session dispatch, delivery comp/cli CLI entry point, hermes_cli/, setup wizard comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint labels May 2, 2026
liuhao1024 added 2 commits May 3, 2026 05:50
1. test_session_create_drops_pending_title_on_valueerror: Lazy session
   creation (c5b4c48) defers DB row to first message, so pending_title
   is no longer cleared during agent build. Update assertion to match.

2. test_dotenv_wins_over_os_environ: Commit 2ef1ad2 changed
   _seed_from_env to prefer .env over os.environ. Rename test and
   update assertion to match new behavior.
@liuhao1024

Copy link
Copy Markdown
Contributor Author

CI Status Update

check-attribution: ✅ PASS (with sunsky.lau@gmail.com mapping added)

test: 2 remaining failures in test_update_yes_flag.py — both are pre-existing upstream failures on main:

  • Upstream main CI (run 25264932001) fails 10+ tests including these same 2
  • This PR fixes 8+ of those upstream failures (TimeoutStopSec, command ordering, SIGKILL behavior, concurrent interrupt, Teams mock)
  • The 2 remaining test_update_yes_flag failures also pass locally on both macOS/Python 3.14 and the PR branch

Summary: This PR is in better shape than upstream main. The 2 remaining test failures are not caused by this PR's changes.

Changes in this PR:

  1. scripts/release.py: Add sunsky.lau@gmail.com to AUTHOR_MAP (fixes check-attribution for all contributor PRs)
  2. tests/acp/test_server.py: Add steer, queue to expected command list
  3. tests/gateway/test_teams.py: Add missing Teams module mocks
  4. tests/hermes_cli/test_gateway_service.py: Update TimeoutStopSec expectations (90→210)
  5. tests/hermes_cli/test_update_gateway_restart.py: Add survivor sweep counter, fix SIGKILL assertions
  6. tests/run_agent/test_concurrent_interrupt.py: Add missing _tool_guardrails attribute
  7. tests/test_tui_gateway_server.py: Fix lazy session creation expectation
  8. tests/tools/test_credential_pool_env_fallback.py: Fix .env precedence test

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the CI-maintenance work. This is an automated hermes-sweeper review; current main already contains or supersedes every correction in this PR.

  • 4009b331f873 independently landed the ACP steer/queue expectation and concurrent-interrupt stub updates; current coverage is at tests/acp/test_server.py:284.
  • f7918c934927 landed the Teams ClientOptions mock now present at tests/gateway/test_teams.py:83 and :152-154.
  • 1f27ca638fd7 corrected the same post-update survivor-sweep mocks; the current production sweep is hermes_cli/main.py:10936-10969.
  • Current main has stronger replacements for the remaining stale assertions: dynamic gateway timeout coverage (tests/hermes_cli/test_gateway_service.py:421-442), dotenv precedence (tests/tools/test_credential_pool_env_fallback.py:110-126), Docker workspace-copy coverage (tests/tools/test_dockerfile_pid1_reaping.py:143-158), and the author mapping (scripts/release.py:62).
  • The lazy-session case was subsequently fixed and reworked in 3b750715a39ed with current regression coverage at tests/test_tui_gateway_server.py:2675-2681.

Closing as implemented on main.

@teknium1 teknium1 closed this Jul 12, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have sweeper:implemented-on-main Sweeper: behavior already present on current main type/test Test coverage or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants