Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fix crashes and enhance test reliability with session IDs #6683

Merged
merged 16 commits into from
Feb 26, 2025

Conversation

ogabrielluiz
Copy link
Contributor

Addressed a crash issue by updating message handling and improved test reliability by adding session ID parameters and fixtures across various tests. Additionally, modified error handling and removed redundant tests to streamline the testing process.

This is a selective rebase of #6672 so we can merge some of the fixes.

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. bug Something isn't working labels Feb 18, 2025
Modify error handling in LCAgentComponent to only delete agent message if it has an ID attribute, preventing potential attribute errors
Modify the test_component_tool_with_api_key test to:
- Use async_start() method for graph execution
- Add session ID to graph
- Improve result verification with vertex result tracking
- Update import path for ChatOutput component
Update test_cycles.py to:
- Add skip markers for tests that now require a LoopComponent
- Improve error message with snapshots for debugging
- Preserve existing test logic while marking as skipped
Add usefixtures decorator to ensure client is available for the tool calling agent test, improving test setup and reliability
Enhance test_component_toolkit.py by adding the client fixture to the test_component_tool_with_api_key method, ensuring proper test setup for API key-dependent scenarios
Add client fixture to the test_agent_component_with_calculator method to ensure proper test setup for API key-dependent scenarios
Remove commented-out test methods for checking required inputs across various components, as these tests were not providing significant value and the inputs are dynamic
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Feb 18, 2025
@ogabrielluiz ogabrielluiz changed the base branch from add-all-makefile to main February 18, 2025 12:29
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Feb 18, 2025
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Feb 18, 2025
@ogabrielluiz ogabrielluiz added the lgtm This PR has been approved by a maintainer label Feb 18, 2025
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Feb 18, 2025
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Feb 18, 2025
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Feb 18, 2025
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Feb 18, 2025
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Feb 18, 2025
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Feb 18, 2025
Copy link

codspeed-hq bot commented Feb 18, 2025

CodSpeed Performance Report

Merging #6683 will improve performances by 42.27%

Comparing fixes-tests (4ee20a2) with main (19ed8bf)

Summary

⚡ 1 improvements
✅ 13 untouched benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
test_build_flow_invalid_job_id 15.3 ms 10.7 ms +42.27%

Copy link
Member

@Cristhianzl Cristhianzl left a comment

Choose a reason for hiding this comment

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

lgtm

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Feb 26, 2025
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Feb 26, 2025
@ogabrielluiz ogabrielluiz added this pull request to the merge queue Feb 26, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 26, 2025
@ogabrielluiz ogabrielluiz added this pull request to the merge queue Feb 26, 2025
Merged via the queue into main with commit d092724 Feb 26, 2025
23 of 31 checks passed
@ogabrielluiz ogabrielluiz deleted the fixes-tests branch February 26, 2025 20:04
Yukiyukiyeah pushed a commit that referenced this pull request Mar 31, 2025
* test: Add session ID parameter to ToolCallingAgentComponent test

* test: Add blocking for langchain_core runnables utility function

* test: Add session ID generation to agent component test

* fix: Safely delete agent message with ID check

Modify error handling in LCAgentComponent to only delete agent message if it has an ID attribute, preventing potential attribute errors

* test: Update ComponentToolkit test to use async start and verify results

Modify the test_component_tool_with_api_key test to:
- Use async_start() method for graph execution
- Add session ID to graph
- Improve result verification with vertex result tracking
- Update import path for ChatOutput component

* test: Skip cycle tests requiring LoopComponent

Update test_cycles.py to:
- Add skip markers for tests that now require a LoopComponent
- Improve error message with snapshots for debugging
- Preserve existing test logic while marking as skipped

* test: Add client fixture to tool calling agent test

Add usefixtures decorator to ensure client is available for the tool calling agent test, improving test setup and reliability

* test: Add client fixture to component tool test with API key

Enhance test_component_toolkit.py by adding the client fixture to the test_component_tool_with_api_key method, ensuring proper test setup for API key-dependent scenarios

* test: Add client fixture to agent component test with calculator

Add client fixture to the test_agent_component_with_calculator method to ensure proper test setup for API key-dependent scenarios

* test: Disable redundant component input tests

Remove commented-out test methods for checking required inputs across various components, as these tests were not providing significant value and the inputs are dynamic

* test: Comment out condition to skip nodes with Tool outputs in setup.py

* run formatter

* test: Re-enable condition to skip nodes with Tool outputs in setup.py

* [autofix.ci] apply automated fixes

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants