Add remaining runtime tests#1824
Merged
chandrams merged 13 commits intokruize:mvp_demofrom Apr 16, 2026
Merged
Conversation
Signed-off-by: Saad Khan <saakhan@ibm.com>
Contributor
Reviewer's GuideRefactors runtime recommendation test setup into a reusable helper module and adds several focused runtime behavior tests while simplifying the original end-to-end test to use the new helpers. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The
_generate_and_list_recommendations_for_tfbhelper tears down the experiment and metric profile but leaves the metadata profile and created layers in place; consider deleting these as well to avoid cross-test interference and accumulated state in the test environment. - In
test_non_runtime_supported_datasource_logs_message_on_generate, relying on a fixedtime.sleep(2)before reading logs may make the test flaky under slow conditions; consider polling for theRUNTIMES_RECOMMENDATIONS_NOT_AVAILABLElog entry with a timeout instead of a hard sleep.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `_generate_and_list_recommendations_for_tfb` helper tears down the experiment and metric profile but leaves the metadata profile and created layers in place; consider deleting these as well to avoid cross-test interference and accumulated state in the test environment.
- In `test_non_runtime_supported_datasource_logs_message_on_generate`, relying on a fixed `time.sleep(2)` before reading logs may make the test flaky under slow conditions; consider polling for the `RUNTIMES_RECOMMENDATIONS_NOT_AVAILABLE` log entry with a timeout instead of a hard sleep.
## Individual Comments
### Comment 1
<location path="tests/scripts/local_monitoring_tests/rest_apis/test_generate_recommendation.py" line_range="68-74" />
<code_context>
+ (e.g., -Xgcpolicy:gencon, -Xgcpolicy:balanced, -Xgcpolicy:optthruput).
+ """
+ list_reco_json = _generate_and_list_recommendations_for_tfb(cluster_type)
+ env_values = _env_values(list_reco_json)
- response = delete_metadata_profile(metadata_profile_name)
- print("delete metadata profile = ", response.status_code)
+ if not _contains_any_pattern(env_values, SEMERU_GC_PATTERNS):
+ pytest.skip("Semeru/OpenJ9 GC policy not detected for current workload – skipping Semeru-specific assertion")
- response = create_metadata_profile(metadata_profile_json_file)
- data = response.json()
- print(data["message"])
- assert response.status_code == SUCCESS_STATUS_CODE
- assert data["status"] == SUCCESS_STATUS
+ assert _contains_any_pattern(env_values, SEMERU_GC_PATTERNS), (
+ f"Expected Semeru GC policy flags {SEMERU_GC_PATTERNS} in JAVA_OPTIONS, got: {env_values}"
+ )
+
</code_context>
<issue_to_address>
**issue (testing):** The Semeru GC policy test contains a redundant assertion that can never fail once the skip condition has passed.
Because the test skips when `env_values` does not contain `SEMERU_GC_PATTERNS`, the final assertion on the same condition can never fail and adds no value. Consider either:
- Dropping the `pytest.skip` and relying on the assertion, or
- Keeping the skip but asserting a stronger condition (for example, that a specific expected flag or env var name is present).
This will ensure the Semeru-specific behavior is actually validated when the test runs.
</issue_to_address>
### Comment 2
<location path="tests/scripts/local_monitoring_tests/rest_apis/test_generate_recommendation.py" line_range="191-200" />
<code_context>
+def test_non_runtime_supported_datasource_logs_message_on_generate(cluster_type):
</code_context>
<issue_to_address>
**suggestion (testing):** The non-runtime-supported datasource test only checks logs; it does not assert the absence of runtime recommendations in the API response.
In `test_non_runtime_supported_datasource_logs_message_on_generate`, you only assert that `RUNTIMES_RECOMMENDATIONS_NOT_AVAILABLE` is logged. To fully verify behavior, also call `list_recommendations(exp_name)` and assert that no GC-related runtime env entries are present (via `_env_values` + `_contains_any_pattern`, or a negative use of `validate_runtime_recommendations_if_present`). This will validate both logging and the recommendation payload for non-runtime-supported datasources.
Suggested implementation:
```python
@pytest.mark.runtimes
def test_non_runtime_supported_datasource_logs_message_on_generate(cluster_type):
"""
Test Description:
For datasources that exist but do NOT support runtime recommendations, generateRecommendations
should still succeed but the server should log RUNTIMES_RECOMMENDATIONS_NOT_AVAILABLE and
the recommendations payload should not contain any runtime GC-related entries.
"""
input_json_file = "../json_files/create_tfb_exp.json"
form_kruize_url(cluster_type)
# Use bulk metadata profile that defines datasources with and without runtime support
delete_and_create_metadata_profile()
# Existing behavior: generate recommendations and assert that the
# RUNTIMES_RECOMMENDATIONS_NOT_AVAILABLE marker is logged.
# NOTE: This assumes the test already creates an experiment and triggers
# generateRecommendations, capturing logs via caplog.
# New behavior: also validate the recommendation payload for this non-runtime-supported datasource.
#
# We expect that there are no GC-related runtime recommendations for this experiment.
# Use the same helper utilities as the other runtime tests to extract env values
# and ensure they don't contain any GC-related patterns.
recommendations = list_recommendations(exp_name)
runtime_env_values = _env_values(recommendations)
assert not _contains_any_pattern(
runtime_env_values,
HOTSPOT_GC_PATTERNS + SEMERU_GC_PATTERNS,
), (
"Non-runtime-supported datasources should not have GC-related runtime recommendations "
"in the listRecommendations payload."
)
```
The edit above assumes a few things about the existing test and helpers:
1. **`exp_name` availability**
- Ensure that `exp_name` is defined earlier in `test_non_runtime_supported_datasource_logs_message_on_generate`, typically when creating the experiment or calling the generate API.
- If `exp_name` is not currently defined, either:
- Parse it from `input_json_file` with an existing helper in this file, or
- Capture it from the response of the experiment creation/generation helper already used in the other tests.
2. **`list_recommendations` signature**
- The code assumes `list_recommendations(exp_name)` exists and returns the recommendation payload for that experiment.
- If your helper has a different name or signature (e.g., takes namespace and experiment name, or returns a list of recs), adapt the call accordingly:
- e.g. `recommendations = list_recommendations([exp_name])` or similar.
3. **`_env_values` helper**
- The snippet assumes that `_env_values(recommendations)` is a valid call that produces a flat list of runtime environment values to be checked against the GC patterns.
- If `_env_values` requires additional parameters (for example, layer names or a key path), adjust the call to mirror how it is used in the other runtime tests in this file.
4. **Location relative to the log assertion**
- If the existing test already has a block that:
- Calls generateRecommendations
- Asserts that `RUNTIMES_RECOMMENDATIONS_NOT_AVAILABLE` is in the logs via `caplog`
- Make sure the newly added recommendation-payload assertions are placed **after** the generate call and log assertion block but before the end of the test, so that:
- The experiment has recommendations available, and
- The log assertion still runs as before.
If any of these assumptions differ from your codebase, keep the overall shape of the added block (call `list_recommendations(exp_name)`, derive env values via `_env_values`, then assert `not _contains_any_pattern(...)`) but adapt it to your actual helper names and data structures.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Signed-off-by: Saad Khan <saakhan@ibm.com>
Signed-off-by: Saad Khan <saakhan@ibm.com>
Contributor
|
@khansaad Have you run these tests? I see these failures - https://ci.app-svc-perf.corp.redhat.com/job/ExternalTeams/job/Autotune/job/kruize_functional_tests/447/ |
Signed-off-by: Saad Khan <saakhan@ibm.com>
Contributor
|
@khansaad Can you please post the latest results ? |
| pushd benchmarks >/dev/null | ||
| echo "5. Installing TechEmpower (Quarkus REST EASY) benchmark into cluster" | ||
| pushd techempower >/dev/null | ||
| kubectl apply -f manifests/default_manifests -n ${APP_NAMESPACE} |
Contributor
There was a problem hiding this comment.
For runtimes specific scripts, we want to use manifests from kruize-demos - which triggers the load by default without an external command which is as part of - kruize/kruize-demos#177
Contributor
Author
|
Contributor
|
@khansaad Why is semeru test skipped here? Can you submit it on jenkins as in the past we have seen failures on jenkins |
chandrams
reviewed
Apr 7, 2026
| pushd benchmarks >/dev/null | ||
| echo "5. Installing TechEmpower (Quarkus REST EASY) benchmark into cluster" | ||
| pushd techempower >/dev/null | ||
| kubectl apply -f manifests/default_manifests -n ${APP_NAMESPACE} |
| @@ -0,0 +1,368 @@ | |||
| """ | |||
| Copyright (c) 2022, 2024 Red Hat, IBM Corporation and others. | |||
Contributor
There was a problem hiding this comment.
Update the copyright year
| @pytest.mark.runtimes | ||
| def test_no_gc_recommendation_when_jvm_version_missing(cluster_type): | ||
| """ | ||
| Test Description: If jvm_info metrics are present but the version label is not part |
Contributor
There was a problem hiding this comment.
Update the test docs with these scenarios
Signed-off-by: Saad Khan <saakhan@ibm.com>
Contributor
|
@kusumachalasani Are you fine with the changes? |
Contributor
I don't see the changes related to manifests using from "manifests/kruize-demos" while deploying benchmarks. |
Signed-off-by: Saad Khan <saakhan@ibm.com>
Signed-off-by: Saad Khan <saakhan@ibm.com>
Contributor
Author
Added now |
Signed-off-by: Saad Khan <saakhan@ibm.com>
|
|
||
| @pytest.mark.skip(reason="This will be enabled once the conditional check PR is merged(PR #1866)") | ||
| @pytest.mark.runtimes | ||
| def test_no_gc_recommendation_when_jvm_version_missing(cluster_type): |
Contributor
There was a problem hiding this comment.
Please make this testcase as positive as the output you are getting as expected and is a default behaviour.
Signed-off-by: Saad Khan <saakhan@ibm.com>
Contributor
Author
Contributor
|
@khansaad I see unexpected test results, but the tests till seem to pass. Can you check & fix these. test_runtime_recommendation - ENV doesn't have quarkus recommendations, but the test passes (this could be due to setup, but test has to fail when quarkus reco is not present) test_no_recommendation_for_layer_runtime_mismatch - Heap & GC recommendations are seen, but the test passes |
chandrams
reviewed
Apr 10, 2026
Signed-off-by: Saad Khan <saakhan@ibm.com>
Contributor
Can you share the latest result links |
Signed-off-by: Saad Khan <saakhan@ibm.com>
Signed-off-by: Saad Khan <saakhan@ibm.com>
Contributor
|
@saad I have tested your changes in PR 1824 - https://ci.app-svc-perf.corp.redhat.com/job/ExternalTeams/job/Autotune/job/kruize_functional_tests/483/. I see 2 issues - Semeru test is skipped on minikube, runtime reco - doesn't have quarkus reco in env, are you enabling it in kube state metrics? |
Signed-off-by: Saad Khan <saakhan@ibm.com>
Contributor
|
@khansaad Please create an issue to investigate the runtimes failures on minikube. |
Contributor
Author
Created 1884 to track this |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR updates the generate_recommendations test to include more runtime recommendations scenarios.
Fixes # (issue)
Type of change
How has this been tested?
Please describe the tests that were run to verify your changes and steps to reproduce. Please specify any test configuration required.
Test Configuration
Checklist 🎯
Additional information
Include any additional information such as links, test results, screenshots here
Summary by Sourcery
Expand runtime recommendations coverage in local monitoring tests by adding scenario-specific test cases and extracting shared runtime helpers into a dedicated utility module.
Enhancements:
Tests: