Skip to content

Conversation

@teocns
Copy link
Contributor

@teocns teocns commented Feb 5, 2025

Some tests are failing due to an inefficiency in the test criteria. Testing for len() of requests makes it fail - every time we change the number of requests fired by the SDK, we would have to change these tests.

  1. Replaced assert len(mock_req.request_history) == N checks with specific request filtering
  2. Added helper to find relevant requests using URL patterns
  3. Made assertions more focused on the actual tool recording functionality
  4. Maintained all existing functionality checks while removing dependency on request count

@teocns teocns requested a review from areibman February 5, 2025 23:29
@codecov
Copy link

codecov bot commented Feb 5, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

@teocns teocns force-pushed the teocns/improve-session-tests-assertions branch from d5b94d4 to 10930f0 Compare February 5, 2025 23:30
Copy link
Contributor

@areibman areibman left a comment

Choose a reason for hiding this comment

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

Broken test but not your fault, approving

…` checks with specific request filtering

1. Replaced `assert len(mock_req.request_history) == N` checks with specific request filtering
2. Added helper to find relevant requests using URL patterns
3. Made assertions more focused on the actual tool recording functionality
4. Maintained all existing functionality checks while removing dependency on request count
… N` checks with specific request filtering

Replaced request count assertions with specific request filtering
Added helper to find relevant requests using URL patterns
Made assertions more focused on the actual action recording
functionality
Maintained all existing functionality checks while removing dependency
on request count

Signed-off-by: Teo <[email protected]>
…ecks with specific request filtering

Signed-off-by: Teo <[email protected]>
…hecks with specific request filtering

Signed-off-by: Teo <[email protected]>
…cks with specific request filtering

Signed-off-by: Teo <[email protected]>
@teocns teocns force-pushed the teocns/improve-session-tests-assertions branch from 10930f0 to 94e077a Compare February 6, 2025 02:09
@teocns teocns merged commit 18a1ce9 into main Feb 6, 2025
10 checks passed
@teocns teocns deleted the teocns/improve-session-tests-assertions branch February 6, 2025 02:10
teocns added a commit that referenced this pull request Feb 6, 2025
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.

3 participants