Skip to content

fix(tests): add use_native_token_count=True when expected#2311

Merged
lizradway merged 1 commit into
strands-agents:mainfrom
lizradway:mcm-integ
May 21, 2026
Merged

fix(tests): add use_native_token_count=True when expected#2311
lizradway merged 1 commit into
strands-agents:mainfrom
lizradway:mcm-integ

Conversation

@lizradway
Copy link
Copy Markdown
Member

Description

Fix test_count_tokens_messages_only integration tests failing across Bedrock, Gemini, and OpenAI model test suites. The tests assert that "native token count" appears in caplog.text, but caplog was empty because the model fixtures were constructed without use_native_token_count=True. Without this flag, count_tokens() early-returns via the heuristic base class path, which never emits the expected debug log.

Added use_native_token_count=True to the model fixtures in all three test classes so the native token counting code path is actually exercised.

Related Issues

Documentation PR

Type of Change

Bug fix

Testing

The fix ensures the test fixtures match the code path the tests are designed to validate. These are integration tests requiring API credentials (AWS, Google, OpenAI) to run end-to-end.

  • I ran hatch run prepare

Checklist

  • I have read the CONTRIBUTING document
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@lizradway lizradway marked this pull request as ready for review May 21, 2026 15:13
@lizradway lizradway deployed to manual-approval May 21, 2026 15:13 — with GitHub Actions Active
@lizradway lizradway enabled auto-merge (squash) May 21, 2026 15:14
@github-actions
Copy link
Copy Markdown

Issue: tests_integ/models/test_model_anthropic.py has the same bug — the TestCountTokens class at line 187 constructs an AnthropicModel without use_native_token_count=True, yet test_count_tokens_messages_only asserts "native token count" in caplog.text. The same early-return via super().count_tokens() applies in AnthropicModel (line 401 of src/strands/models/anthropic.py).

Suggestion: Add use_native_token_count=True to the AnthropicModel fixture in tests_integ/models/test_model_anthropic.py as well to fix the same issue consistently across all model providers.

@github-actions
Copy link
Copy Markdown

Assessment: Comment

The fix is correct and minimal — the model fixtures now match the code path the tests are designed to validate.

Review Details
  • Missing provider: tests_integ/models/test_model_anthropic.py has the identical bug (asserts on "native token count" without setting use_native_token_count=True). Consider including it in this PR for a complete fix across all providers.
  • Checklist items: The I ran hatch run prepare and I have read the CONTRIBUTING document boxes are unchecked — please confirm these before merge.

Clean, well-scoped fix — just flagging the missing Anthropic case for completeness.

@lizradway lizradway merged commit 64a6862 into strands-agents:main May 21, 2026
40 of 52 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants