-
Notifications
You must be signed in to change notification settings - Fork 55
LCORE-760: fix hardcoded model and provider in e2e tests #680
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import time | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import jsonschema | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from typing import Any | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from behave.runner import Context | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def normalize_endpoint(endpoint: str) -> str: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -141,3 +142,19 @@ def restart_container(container_name: str) -> None: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Wait for container to be healthy | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| wait_for_container_health(container_name) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def replace_placeholders(context: Context, text: str) -> str: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Replace {MODEL} and {PROVIDER} placeholders with actual values from context. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Args: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| context: Behave context containing default_model and default_provider | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| text: String that may contain {MODEL} and {PROVIDER} placeholders | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Returns: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String with placeholders replaced by actual values | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| result = text.replace("{MODEL}", context.default_model) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| result = result.replace("{PROVIDER}", context.default_provider) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return result | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+147
to
+160
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add defensive attribute checks. The function accesses Apply this diff to add defensive checks: def replace_placeholders(context: Context, text: str) -> str:
"""Replace {MODEL} and {PROVIDER} placeholders with actual values from context.
Args:
context: Behave context containing default_model and default_provider
text: String that may contain {MODEL} and {PROVIDER} placeholders
Returns:
String with placeholders replaced by actual values
+
+ Raises:
+ AttributeError: If context is missing default_model or default_provider
"""
+ if not hasattr(context, 'default_model') or not hasattr(context, 'default_provider'):
+ raise AttributeError(
+ "Context missing required attributes: default_model and/or default_provider. "
+ "Ensure before_all in environment.py executed successfully."
+ )
+
result = text.replace("{MODEL}", context.default_model)
result = result.replace("{PROVIDER}", context.default_provider)
return result📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove dead exception handler.
The
exceptblock catchesKeyError, but the code uses.get()method (lines 37-42) which returnsNoneinstead of raisingKeyError. This makes theKeyErrorhandler unreachable.Apply this diff:
📝 Committable suggestion
🤖 Prompt for AI Agents