Skip to content

chore: fix vision test by mocking aux client#803

Closed
OutThisLife wants to merge 2 commits into
mainfrom
fix/vision-test-flake
Closed

chore: fix vision test by mocking aux client#803
OutThisLife wants to merge 2 commits into
mainfrom
fix/vision-test-flake

Conversation

@OutThisLife

Copy link
Copy Markdown
Collaborator

What does this PR do?

test_analysis_error_logs_exc_info fails in CI but passes locally. The test doesn't mock _aux_async_client, so when another test leaves that global as None, the function early-returns before reaching the except block — no error is ever logged, assertion fails.

Related Issue

N/A

Type of Change

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

Changes Made

  • tests/tools/test_vision_tools.py: mock _aux_async_client to a truthy value so the code reaches the download/exception path
  • Tightened assertion from r.exc_info is not None to r.exc_info and r.exc_info[0] is not None (the former passes even when there's no real exception)

How to Test

  1. python -m pytest tests/tools/test_vision_tools.py::TestErrorLoggingExcInfo -xvs
  2. Run the full suite a few times to confirm no ordering-dependent flake

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • 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 make check (lint + test) and all checks pass
  • I've added tests for my changes — N/A (this fixes an existing test)
  • I've tested on my platform: Ubuntu 24.04 (WSL2)

Documentation & Housekeeping

  • N/A — no docs, config, or schema changes

@OutThisLife OutThisLife requested a review from teknium1 March 10, 2026 17:44
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the fix @OutThisLife! This was already independently fixed on main in commit 0229e6b by @SHL0MS (same approach — mocking _aux_async_client and DEFAULT_VISION_MODEL). Closing as redundant, but appreciate you catching and reporting the flaky test.

@teknium1 teknium1 closed this Mar 11, 2026
teknium1 added a commit that referenced this pull request Mar 11, 2026
The weaker assertion (r.exc_info is not None) passes even when
exc_info is (None, None, None). Check r.exc_info[0] is not None
to verify actual exception info is present.

The _aux_async_client mock was already applied on main.

Co-authored-by: OutThisLife <nickolasgustafsson@gmail.com>
@teknium1

Copy link
Copy Markdown
Contributor

The _aux_async_client mock was already on main (reformatted to PEP8 parens in a prior commit). Your tighter exc_info assertion (r.exc_info and r.exc_info[0] is not None) has been applied in 184aa5b. Thanks — co-authored-by credit included.

angelburgosrosado pushed a commit to angelburgosrosado/hermes-agent that referenced this pull request Apr 27, 2026
)

The weaker assertion (r.exc_info is not None) passes even when
exc_info is (None, None, None). Check r.exc_info[0] is not None
to verify actual exception info is present.

The _aux_async_client mock was already applied on main.

Co-authored-by: OutThisLife <nickolasgustafsson@gmail.com>
02356abc pushed a commit to 02356abc/hermes-agent that referenced this pull request May 14, 2026
)

The weaker assertion (r.exc_info is not None) passes even when
exc_info is (None, None, None). Check r.exc_info[0] is not None
to verify actual exception info is present.

The _aux_async_client mock was already applied on main.

Co-authored-by: OutThisLife <nickolasgustafsson@gmail.com>
olympus-terminal pushed a commit to olympus-terminal/hermes-agent that referenced this pull request May 16, 2026
)

The weaker assertion (r.exc_info is not None) passes even when
exc_info is (None, None, None). Check r.exc_info[0] is not None
to verify actual exception info is present.

The _aux_async_client mock was already applied on main.

Co-authored-by: OutThisLife <nickolasgustafsson@gmail.com>
Egavasyug pushed a commit to Egavasyug/hermes-agent that referenced this pull request Jun 10, 2026
)

The weaker assertion (r.exc_info is not None) passes even when
exc_info is (None, None, None). Check r.exc_info[0] is not None
to verify actual exception info is present.

The _aux_async_client mock was already applied on main.

Co-authored-by: OutThisLife <nickolasgustafsson@gmail.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