Add ENV_CONFIGS support for evaluation tests & fix kubectl tabular query filtering - #1471
Conversation
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
Signed-off-by: Tomer Keshet <tomer@robusta.dev>
|
|
✅ Docker image ready for
Use this tag to pull the image for testing. 📋 Copy commandsgcloud auth configure-docker us-central1-docker.pkg.dev
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:09b55fc
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:09b55fc me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:09b55fc
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:09b55fcPatch Helm values in one line (choose the chart you use): HolmesGPT chart: helm upgrade --install holmesgpt ./helm/holmes \
--set registry=me-west1-docker.pkg.dev/robusta-development/development \
--set image=holmes-dev:09b55fcRobusta wrapper chart: helm upgrade --install robusta robusta/robusta \
--reuse-values \
--set holmes.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set holmes.image=holmes-dev:09b55fc |
✅ Results of HolmesGPT evalsAutomatically triggered by commit e6d63fa on branch Results of HolmesGPT evals
📖 Legend
🔄 Re-run evals manually
Option 1: Comment on this PR with Or with more options (one per line): Run evals on a different branch (e.g., master) for comparison:
Quick re-run: Use Option 2: Trigger via GitHub Actions UI → "Run workflow" 🏷️ Valid markers
Commands: CLI: |
WalkthroughThe pull request introduces environment configuration support to the test suite, allowing tests to run against multiple environment configurations. It refactors test utilities to support parameterized env_config, updates reporting to track and compare results across configurations, and marks several test fixtures as skipped or updates their metadata. Additionally, Kubernetes toolset filtering is improved to preserve table headers. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Deploy Preview for holmes-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🔬 CLI Performance Benchmark🟡 Startup Time (no LLM)Measures
🟡 Full CLI with LLMMeasures
PR: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/llm/fixtures/test_ask_holmes/50a_logs_since_last_specific_month/test_case.yaml (1)
16-22:⚠️ Potential issue | 🟡 MinorFix grammar in
skip_reason.Minor wording tweak for clarity.
✏️ Proposed fix
-skip_reason: "this test fails now as the toolset are not consistent with the mocked data" +skip_reason: "this test fails now as the toolset is not consistent with the mocked data"tests/llm/conftest.py (1)
845-862:⚠️ Potential issue | 🟡 MinorCapture
env_configvia pytest hooks instead of hardcoding "default"
Skipped tests always report"default", which hides real parameters. Inpytest_runtest_makereportappenditem.callspec.paramstoreport.user_properties, then in the skipped-test branch extractenv_config(falling back to"default").Example
# in pytest_runtest_makereport report.user_properties.append(("callspec_params", getattr(item.callspec, "params", {}))) # in pytest_runtest_logreport for skipped tests params = dict(report.user_properties).get("callspec_params", {}) env_val = params.get("env_config", "default") env_config = getattr(env_val, "name", str(env_val)) "env_config": env_config
🤖 Fix all issues with AI agents
In `@holmes/plugins/toolsets/kubernetes.yaml`:
- Line 239: The kubectl command line in the command field that uses {{
filter_pattern }} currently lets grep's exit code 1 (no matches) fail the whole
tool; update the subshell that runs read/echo/grep so that grep returning no
matches is treated as success by making grep non-fatal (for example, append an
OR that forces a zero exit status on grep failures) — change the command that
includes {{ kind }}, {{ columns }} and {{ filter_pattern }} to wrap the grep
accordingly so only real errors (not "no matches") cause the tool to fail.
In
`@tests/llm/fixtures/test_ask_holmes/91f_datadog_logs_historical_pod/test_case.yaml`:
- Around line 20-21: The YAML comment references a non-existent pytest marker
"benchmark"; either remove the commented lines in test_case.yaml that reference
"benchmark" or add a proper marker declaration for "benchmark" in pyproject.toml
under the pytest markers (e.g., add "benchmark: description" to the markers list
in [tool.pytest.ini_options]) so the marker is valid for future use.
In
`@tests/llm/fixtures/test_ask_holmes/93_events_since_specific_date/test_case.yaml`:
- Around line 10-13: Fix the typo in the YAML test metadata: update the
skip_reason value for the test (the YAML key skip_reason) to correct the
duplicated word "to to" -> "to" so the string reads "this sometimes fails due to
missing mock errors"; ensure only the value for skip_reason is changed and
formatting remains valid YAML.
In `@tests/llm/utils/commands.py`:
- Around line 8-15: Add the missing typing import and return annotations for the
context managers: import Iterator from typing (e.g. change the top import to
"from typing import TYPE_CHECKING, Dict, Optional, Iterator") and update each
context manager function in this file to declare a return type of "->
Iterator[None]" (ensure functions that use yield are the ones annotated). Keep
the rest of the imports (is_run_live_enabled, HolmesTestCase) unchanged and only
add the Iterator import and the "-> Iterator[None]" annotations to the context
manager definitions.
🧹 Nitpick comments (4)
tests/llm/fixtures/test_ask_holmes/44_slack_statefulset_logs/test_case.yaml (1)
14-15: Consider linking the skip to a tracking issue or expiry.
Skipping is fine short-term, but a reference helps ensure the eval is revisited.tests/llm/utils/reporting/terminal_reporter.py (1)
191-242: Hoist env_config uniqueness check out of the row loop.The env_config set is recomputed for every row; precomputing once reduces repeated work and keeps the logic centralized.
🔧 Suggested refactor
- # Add rows to table - for result in sorted_results: + # Add rows to table + unique_env_configs = {r.get("env_config", "default") for r in sorted_results} + show_env_config = len(unique_env_configs) > 1 + for result in sorted_results: status = TestStatus(result) @@ - unique_env_configs = {r.get("env_config", "default") for r in sorted_results} - if len(unique_env_configs) > 1: + if show_env_config: parts.append(env_config)tests/llm/test_investigate.py (1)
85-87: Add a return type and clarify the docstring intent.This keeps type-hint coverage consistent and documents why the IDs exist (stable pytest names).
✍️ Suggested update
-def _get_env_config_ids(): - """Generate ids for env_config parameterization.""" +def _get_env_config_ids() -> list[str]: + """Keep pytest parametrized IDs stable by using env_config names.""" return [ec.name for ec in get_env_configs()]As per coding guidelines, "Use type hints throughout Python code and run 'mypy' for type checking" and "Write clear, concise comments that explain 'why' rather than 'what'."
tests/llm/test_ask_holmes.py (1)
53-55: Add a return type and clarify the docstring intent.This keeps typing consistent and explains why the IDs are derived from env_config names.
✍️ Suggested update
-def _get_env_config_ids(): - """Generate ids for env_config parameterization.""" +def _get_env_config_ids() -> list[str]: + """Keep pytest parametrized IDs stable by using env_config names.""" return [ec.name for ec in get_env_configs()]As per coding guidelines, "Use type hints throughout Python code and run 'mypy' for type checking" and "Write clear, concise comments that explain 'why' rather than 'what'."
2 participants
Title:
Add ENV_CONFIGS support for evaluation tests & fix kubectl tabular query filtering
Description:
Summary
ENV_CONFIGS Feature
Enables comparing test runs across different environment configurations (similar to multi-model support).
Format: