doc: minor improvements in documentation #395
Open
Conversation
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
Signed-off-by: Milind Waykole <[email protected]>
📝 WalkthroughSummary by CodeRabbit
Summary by CodeRabbit
WalkthroughThe documentation was updated to expand instructions for running categorized test suites using pytest markers and to clarify the usage of test case parameters. Additional deployment modes and options for upgrade testing were documented, along with minor spelling corrections and clarifications in example commands. Changes
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
The following are automatically added/executed:
Available user actions:
Supported labels{'/lgtm', '/build-push-pr-image', '/hold', '/wip', '/verified', '/cherry-pick'} |
for more information, see https://pre-commit.ci
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
docs/GETTING_STARTED.md (3)
61-69: Quote the marker and clarify placeholder pathThe command will break in
zsh/fishor when<marker>is a composite expression (smoke or sanity) unless it is quoted, and the placeholder path should follow repository layout conventions.-uv run pytest -m <marker> ./test/path-of-component +uv run pytest -m "<marker>" tests/<component-path>Consider explicitly mentioning that
<marker>should be one ofsmoke,sanity,tier1, ortier2.
72-83: Minor grammar & quoting tweaks in examples
eg:→e.g.:- Quote multi-word expressions to avoid shell globbing / word-splitting.
- For AND conditions pytest requires parentheses when mixing AND/OR; showing them prevents user pitfalls.
-eg: +e.g.: @@ -uv run pytest -m "smoke and serverless" +uv run pytest -m '(smoke and serverless)'
94-99: Fix article & streamline operator-filter exampleLanguageTool hint is correct—an article is missing, and the example sentence is long. Also, wrap the empty string in single quotes to avoid YAML/INI escaping issues when the command is copy-pasted.
-By default, `Serverless` (serverless-operator), `Authorino` (authorino-operator) and `Service Mesh` (servicemeshoperator) operators are checked to be installed when -running model server Serverless tests. -To check only specific operator, pass `--tc=dependent_operators:<operator_name>` to pytest. +By default, the `Serverless` (serverless-operator), `Authorino` (authorino-operator), and `Service Mesh` (servicemeshoperator) operators must be installed when +running model-server Serverless tests. +To check only a specific operator, pass `--tc=dependent_operators:<operator_name>` to pytest. @@ -When you don't want to check any dependent operators, pass `--tc=dependent_operators:""`. +When you don't want to check any dependent operators, pass: +```bash +--tc=dependent_operators:'' +```
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/GETTING_STARTED.md(2 hunks)docs/UPGRADE.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
docs/UPGRADE.md (1)
Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.
docs/GETTING_STARTED.md (4)
Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#283
File: tests/model_registry/test_user_permission.py:31-31
Timestamp: 2025-05-05T13:48:02.409Z
Learning: In the opendatahub-tests repository, bypassing TLS verification with `--insecure-skip-tls-verify=true` is an acceptable practice in test environments, particularly in the model registry tests.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/test_mr_rbac.py:64-77
Timestamp: 2025-06-16T11:26:53.789Z
Learning: In Model Registry RBAC tests, client instantiation tests are designed to verify the ability to create and use the MR python client, with actual API functionality testing covered by separate existing tests.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:166-175
Timestamp: 2025-06-16T11:25:39.599Z
Learning: In tests/model_registry/rbac/conftest.py, predictable names are intentionally used for test resources (like RoleBindings and groups) instead of random names. This design choice prioritizes exposing cleanup failures from previous test runs through name collisions rather than masking such issues with random names. The philosophy is that test failures should be observable and informative to help debug underlying infrastructure or cleanup issues.
🪛 LanguageTool
docs/GETTING_STARTED.md
[uncategorized] ~96-~96: Possible missing article found.
Context: ... server Serverless tests. To check only specific operator, pass `--tc=dependent_operator...
(AI_HYDRA_LEO_MISSING_A)
dbasunag
reviewed
Jun 30, 2025
rhods-ci-bot
added
commented-by-mwaykole
commented-by-dbasunag
and removed
commented-by-mwaykole
commented-by-dbasunag
labels
Signed-off-by: Milind Waykole <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
docs/GETTING_STARTED.md (2)
84-91: Minor wording / punctuation nit-eg: +e.g.:Everything else in this section reads well.
103-107: Grammar tweak for clarity-To check only specific operator, pass `--tc=dependent_operators:<operator_name>` to pytest. +To check only a specific operator, pass `--tc=dependent_operators:<operator_name>` to pytest.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/GETTING_STARTED.md(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
docs/GETTING_STARTED.md (4)
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#283
File: tests/model_registry/test_user_permission.py:31-31
Timestamp: 2025-05-05T13:48:02.409Z
Learning: In the opendatahub-tests repository, bypassing TLS verification with `--insecure-skip-tls-verify=true` is an acceptable practice in test environments, particularly in the model registry tests.
Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/test_mr_rbac.py:64-77
Timestamp: 2025-06-16T11:26:53.789Z
Learning: In Model Registry RBAC tests, client instantiation tests are designed to verify the ability to create and use the MR python client, with actual API functionality testing covered by separate existing tests.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:166-175
Timestamp: 2025-06-16T11:25:39.599Z
Learning: In tests/model_registry/rbac/conftest.py, predictable names are intentionally used for test resources (like RoleBindings and groups) instead of random names. This design choice prioritizes exposing cleanup failures from previous test runs through name collisions rather than masking such issues with random names. The philosophy is that test failures should be observable and informative to help debug underlying infrastructure or cleanup issues.
🪛 LanguageTool
docs/GETTING_STARTED.md
[uncategorized] ~105-~105: Possible missing article found.
Context: ... server Serverless tests. To check only specific operator, pass `--tc=dependent_operator...
(AI_HYDRA_LEO_MISSING_A)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (1)
docs/GETTING_STARTED.md (1)
71-79: Nice addition – ignore examples address prior feedbackThe new
--ignoreexamples clearly illustrate single vs. multiple directory exclusion and reflect the earlier review discussion.
Looks good as-is.
Signed-off-by: Milind Waykole <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
docs/GETTING_STARTED.md (1)
66-69: Fix incorrect path in example commandThe repository uses a top-level
tests/directory. The current example still points to a non-existenttest/path, which will mislead users.-uv run pytest -m <marker> ./test/path-of-component +uv run pytest -m <marker> tests/<component_path>-You can omit `./test/path-of-component` if you want to run markers for all components. +You can omit `tests/<component_path>` if you want to run the marker across all components.
🧹 Nitpick comments (3)
docs/GETTING_STARTED.md (3)
62-63: Tighten wording for clarity-The test suite is organized into different markers, and directories are arranged based on their components: +The test suite is organized using pytest markers, and test directories are grouped by component:
84-85: Replace informal “eg:” with standard “e.g.”-eg: +e.g.:
103-104: Streamline sentence and remove duplicate “Serverless”-By default, `Serverless` (serverless-operator), `Authorino` (authorino-operator) and `Service Mesh` (servicemeshoperator) operators are checked to be installed when -running model server Serverless tests. +By default, the presence of `Serverless` (serverless-operator), `Authorino` (authorino-operator), and `Service Mesh` (servicemeshoperator) operators is verified when running model-server tests.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/GETTING_STARTED.md(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
docs/GETTING_STARTED.md (4)
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#283
File: tests/model_registry/test_user_permission.py:31-31
Timestamp: 2025-05-05T13:48:02.409Z
Learning: In the opendatahub-tests repository, bypassing TLS verification with `--insecure-skip-tls-verify=true` is an acceptable practice in test environments, particularly in the model registry tests.
Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/test_mr_rbac.py:64-77
Timestamp: 2025-06-16T11:26:53.789Z
Learning: In Model Registry RBAC tests, client instantiation tests are designed to verify the ability to create and use the MR python client, with actual API functionality testing covered by separate existing tests.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:166-175
Timestamp: 2025-06-16T11:25:39.599Z
Learning: In tests/model_registry/rbac/conftest.py, predictable names are intentionally used for test resources (like RoleBindings and groups) instead of random names. This design choice prioritizes exposing cleanup failures from previous test runs through name collisions rather than masking such issues with random names. The philosophy is that test failures should be observable and informative to help debug underlying infrastructure or cleanup issues.
🪛 LanguageTool
docs/GETTING_STARTED.md
[uncategorized] ~105-~105: Possible missing article found.
Context: ... server Serverless tests. To check only specific operator, pass `--tc=dependent_operator...
(AI_HYDRA_LEO_MISSING_A)
Jooho
reviewed
Jun 30, 2025
Jooho
reviewed
Jun 30, 2025
| running model server Serverless tests. | ||
| To check only specific operator, pass `--tc=dependent_operators:<operator_name>` to pytest. | ||
| For example, to check only `Serveless` and `Service Mesh` operators, pass `--tc=dependent_operators:serverless-operator,servicemeshoperator`. | ||
| For example, to check only `Serverless` and `Service Mesh` operators, pass `--tc=dependent_operators:serverless-operator,servicemeshoperator`. |
There was a problem hiding this comment.
I think it would be helpful to show a matrix.
For example, when using Serverless, we can list the operators that should be installed and checked. Likewise for RawDeployment, and for each individual component, it would be good to highlight the key things that need to be verified.
Also, for something like Kueue, there are multiple installation methods, and I understand that some configurations need to be modified as well.
Jooho
reviewed
Jun 30, 2025
docs/UPGRADE.md
Outdated
| @@ -38,5 +38,9 @@ uv run pytest --pre-upgrade --post-upgrade --upgrade-deployment-modes=servelerss | |||
| ``` | |||
|
|
|||
| ```bash | |||
| uv run pytest --pre-upgrade --post-upgrade --upgrade-deployment-modes=servelerss,rawdeployment | |||
| uv run pytest --pre-upgrade --post-upgrade --upgrade-deployment-modes=servelerss,rawdeployment,authorino | |||
There was a problem hiding this comment.
Could you please add an explanation of what this command does?
I think it would be helpful to understand what the command is actually doing.
Jooho
reviewed
Jun 30, 2025
docs/UPGRADE.md
Outdated
| uv run pytest --pre-upgrade --post-upgrade --upgrade-deployment-modes=servelerss,rawdeployment | ||
| uv run pytest --pre-upgrade --post-upgrade --upgrade-deployment-modes=servelerss,rawdeployment,authorino | ||
| ``` | ||
| If you need to upgrade a raw cluster with no dependent operators: |
There was a problem hiding this comment.
I'm a bit confused about what this means.
What does it mean to upgrade a raw cluster with no dependent operators?
Could you please explain what it implies to update a cluster without any dependent operators?
dbasunag
reviewed
Jun 30, 2025
docs/GETTING_STARTED.md
Outdated
| ```bash | ||
| uv run pytest -m <marker> ./test/path-of-component | ||
| ``` | ||
| You can omit `./test/path-of-component` if you want to run markers for all components. |
There was a problem hiding this comment.
ditto. Also please move this line to section #### Excluding Test Suites
There was a problem hiding this comment.
if there is no need to include ./ ; the same update should be included in the next line
Signed-off-by: Milind Waykole <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
docs/GETTING_STARTED.md (4)
61-70: Remove redundant./prefix & use a clearer placeholder.
pytestpaths are already interpreted relative to CWD, so the leading./is unnecessary noise.
While you are touching the line, consider using a more explicit placeholder than the rather informalpath-of-component.-uv run pytest -m <marker> ./tests/path-of-component +uv run pytest -m <marker> tests/<component_path>
71-76: Heading/comments mismatch & missing trailing slash.The comment says “Ignore a single test suite” but the brace-expansion ignores two.
Also, adding a trailing slash makes it evident these are directories (and matches earlier discussion).-# Ignore a single test suite -uv run pytest --ignore=tests/{model_registry,workbenches} +# Ignore multiple test suites +uv run pytest --ignore=tests/{model_registry,workbenches}/
81-88: Minor style fixes (e.g. → e.g., shell quoting).-eg: +e.g.: -uv run pytest -m "smoke or sanity" +uv run pytest -m 'smoke or sanity' -uv run pytest -m "smoke and serverless" +uv run pytest -m 'smoke and serverless'Quoting with single quotes avoids unintended shell globbing/variable expansion.
100-104: Tighten wording & resolve grammar issues.
- “model server Serverless tests” is awkward duplication.
- Static-analysis flagged a missing article.
-By default, `Serverless` (serverless-operator), `Authorino` (authorino-operator) and `Service Mesh` (servicemeshoperator) operators are checked to be installed when -running model server Serverless tests. +By default, the `Serverless` (serverless-operator), `Authorino` (authorino-operator), and `Service Mesh` (servicemeshoperator) operators are verified as installed when running model-server tests in *Serverless* mode.(While here you might also clarify that an empty value can be supplied via
--tc=dependent_operators:which avoids shell-quoting headaches.)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/GETTING_STARTED.md(2 hunks)docs/UPGRADE.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/UPGRADE.md
🧰 Additional context used
🧠 Learnings (1)
docs/GETTING_STARTED.md (9)
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#283
File: tests/model_registry/test_user_permission.py:31-31
Timestamp: 2025-05-05T13:48:02.409Z
Learning: In the opendatahub-tests repository, bypassing TLS verification with `--insecure-skip-tls-verify=true` is an acceptable practice in test environments, particularly in the model registry tests.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#338
File: tests/model_registry/rbac/test_mr_rbac.py:24-53
Timestamp: 2025-06-06T12:22:57.057Z
Learning: In the opendatahub-tests repository, prefer keeping test parameterization configurations inline rather than extracting them to separate variables/constants, as it makes triaging easier by avoiding the need to jump between different parts of the file to understand the test setup.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#240
File: conftest.py:328-335
Timestamp: 2025-05-13T17:27:26.289Z
Learning: In the opendatahub-tests codebase, py_config["must_gather_collector"]["must_gather_base_directory"] is never empty as it's set during pytest_sessionstart() by combining get_base_dir() with BASE_DIRECTORY_NAME="must-gather-collected", making it safe to use in directory operations.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#240
File: conftest.py:328-335
Timestamp: 2025-05-13T17:27:26.289Z
Learning: In the opendatahub-tests codebase, py_config["must_gather_collector"]["must_gather_base_directory"] is never empty as it's set during pytest_sessionstart() by combining get_base_dir() with BASE_DIRECTORY_NAME, making it safe to use in directory operations.
Learnt from: brettmthompson
PR: opendatahub-io/opendatahub-tests#269
File: tests/model_serving/model_server/serverless/test_scale_to_zero.py:0-0
Timestamp: 2025-04-29T00:49:40.918Z
Learning: In test cases for the opendatahub-tests repository, when verifying specific behaviors resulting from a sequence of operations, hard-coded validation values like "configurationGeneration=3" can be appropriate assertions to validate expected states, especially when they represent an invariant that should remain consistent after specific operations.
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#320
File: tests/model_registry/rest_api/conftest.py:200-216
Timestamp: 2025-06-05T14:32:40.247Z
Learning: In the opendatahub-tests repository, the test fixtures should raise exceptions on cleanup failures rather than just logging warnings. The user fege prefers strict cleanup behavior where tests fail if cleanup doesn't work properly, rather than silently continuing.
Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/test_mr_rbac.py:64-77
Timestamp: 2025-06-16T11:26:53.789Z
Learning: In Model Registry RBAC tests, client instantiation tests are designed to verify the ability to create and use the MR python client, with actual API functionality testing covered by separate existing tests.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:166-175
Timestamp: 2025-06-16T11:25:39.599Z
Learning: In tests/model_registry/rbac/conftest.py, predictable names are intentionally used for test resources (like RoleBindings and groups) instead of random names. This design choice prioritizes exposing cleanup failures from previous test runs through name collisions rather than masking such issues with random names. The philosophy is that test failures should be observable and informative to help debug underlying infrastructure or cleanup issues.
🪛 LanguageTool
docs/GETTING_STARTED.md
[uncategorized] ~102-~102: Possible missing article found.
Context: ... server Serverless tests. To check only specific operator, pass `--tc=dependent_operator...
(AI_HYDRA_LEO_MISSING_A)
dbasunag
reviewed
Jul 1, 2025
|
|
||
| #### Smoke/Sanity/Tier1/Tier2 Tests | ||
| ```bash | ||
| uv run pytest -m <marker> ./tests/path-of-component |
There was a problem hiding this comment.
Tests should be required to run from root folder - opendatahub-tests. The command would be `uv run pytest -m tests/path-of-component. We won't have any tier1 tests in this repository. The marker should be removed.
| ```bash | ||
| uv run pytest -m <marker> ./tests/path-of-component | ||
| ``` | ||
| You can omit `./tests/path-of-component` if you want to run markers for all components. |
There was a problem hiding this comment.
ditto. path should be tests/path-of-component.
Signed-off-by: Milind Waykole <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
docs/GETTING_STARTED.md (3)
75-75: Ensure the ignore pattern matches directoriesThe trailing slash is required for pytest to recognise the arguments as directories; omitting it may falsely ignore files named
model_registry*.-uv run pytest --ignore=tests/{model_registry,workbenches} +uv run pytest --ignore=tests/{model_registry,workbenches}/
81-81: Minor typo – use “e.g.”-eg: +e.g.:
100-104: Tighten wording & fix minor grammar issues-By default, `Serverless` (serverless-operator), `Authorino` (authorino-operator) and `Service Mesh` (servicemeshoperator) operators are checked to be installed when -running model server Serverless tests. -To check only specific operator, pass `--tc=dependent_operators:<operator_name>` to pytest. -For example, to check only `Serverless` and `Service Mesh` operators, pass `--tc=dependent_operators:serverless-operator,servicemeshoperator`. -When you don't want to check any dependent operators, pass `--tc=dependent_operators:""`. +By default, the tests verify that `Serverless` (serverless-operator), `Authorino` (authorino-operator), and `Service Mesh` (servicemeshoperator) operators are installed when +running model-server Serverless tests. +To check only a specific operator, pass `--tc=dependent_operators:<operator_name>` to pytest. +For example, to check only the `Serverless` and `Service Mesh` operators, pass `--tc=dependent_operators:serverless-operator,servicemeshoperator`. +If you do not want to check any dependent operators, pass `--tc=dependent_operators:""`.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/GETTING_STARTED.md(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
docs/GETTING_STARTED.md (11)
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#283
File: tests/model_registry/test_user_permission.py:31-31
Timestamp: 2025-05-05T13:48:02.409Z
Learning: In the opendatahub-tests repository, bypassing TLS verification with `--insecure-skip-tls-verify=true` is an acceptable practice in test environments, particularly in the model registry tests.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#338
File: tests/model_registry/rbac/test_mr_rbac.py:24-53
Timestamp: 2025-06-06T12:22:57.057Z
Learning: In the opendatahub-tests repository, prefer keeping test parameterization configurations inline rather than extracting them to separate variables/constants, as it makes triaging easier by avoiding the need to jump between different parts of the file to understand the test setup.
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#320
File: tests/model_registry/rest_api/conftest.py:200-216
Timestamp: 2025-06-05T14:32:40.247Z
Learning: In the opendatahub-tests repository, the test fixtures should raise exceptions on cleanup failures rather than just logging warnings. The user fege prefers strict cleanup behavior where tests fail if cleanup doesn't work properly, rather than silently continuing.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#240
File: conftest.py:328-335
Timestamp: 2025-05-13T17:27:26.289Z
Learning: In the opendatahub-tests codebase, py_config["must_gather_collector"]["must_gather_base_directory"] is never empty as it's set during pytest_sessionstart() by combining get_base_dir() with BASE_DIRECTORY_NAME, making it safe to use in directory operations.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#240
File: conftest.py:328-335
Timestamp: 2025-05-13T17:27:26.289Z
Learning: In the opendatahub-tests codebase, py_config["must_gather_collector"]["must_gather_base_directory"] is never empty as it's set during pytest_sessionstart() by combining get_base_dir() with BASE_DIRECTORY_NAME="must-gather-collected", making it safe to use in directory operations.
Learnt from: brettmthompson
PR: opendatahub-io/opendatahub-tests#269
File: tests/model_serving/model_server/serverless/test_scale_to_zero.py:0-0
Timestamp: 2025-04-29T00:49:40.918Z
Learning: In test cases for the opendatahub-tests repository, when verifying specific behaviors resulting from a sequence of operations, hard-coded validation values like "configurationGeneration=3" can be appropriate assertions to validate expected states, especially when they represent an invariant that should remain consistent after specific operations.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#296
File: tests/model_registry/rbac/test_mr_rbac_sa.py:1-4
Timestamp: 2025-05-06T23:36:04.545Z
Learning: The opendatahub-tests repository is pinned to Python 3.12, which supports `typing.Self` natively, so there's no need for backward compatibility imports from typing_extensions.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:355-371
Timestamp: 2025-06-16T11:25:51.461Z
Learning: In the opendatahub-tests repository, the login_as_test_user fixture in tests/model_registry/rbac/conftest.py intentionally logs back in as the original admin user without providing a password using login_with_user_password. This behavior is acceptable in their testing environment.
Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/test_mr_rbac.py:64-77
Timestamp: 2025-06-16T11:26:53.789Z
Learning: In Model Registry RBAC tests, client instantiation tests are designed to verify the ability to create and use the MR python client, with actual API functionality testing covered by separate existing tests.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:166-175
Timestamp: 2025-06-16T11:25:39.599Z
Learning: In tests/model_registry/rbac/conftest.py, predictable names are intentionally used for test resources (like RoleBindings and groups) instead of random names. This design choice prioritizes exposing cleanup failures from previous test runs through name collisions rather than masking such issues with random names. The philosophy is that test failures should be observable and informative to help debug underlying infrastructure or cleanup issues.
🪛 LanguageTool
docs/GETTING_STARTED.md
[uncategorized] ~102-~102: Possible missing article found.
Context: ... server Serverless tests. To check only specific operator, pass `--tc=dependent_operator...
(AI_HYDRA_LEO_MISSING_A)
Comment on lines
+67
to
+69
| uv run pytest -m <marker> ./tests/path-of-component | ||
| ``` | ||
| You can omit `./tests/path-of-component` if you want to run markers for all components. |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Align path example with agreed repository conventions
Prior feedback established that commands should reference tests/path-of-component (no leading ./). Retaining ./ will re-introduce inconsistency with the rest of the docs and reviewers’ guidance.
-uv run pytest -m <marker> ./tests/path-of-component
+uv run pytest -m <marker> tests/path-of-component
-
-You can omit `./tests/path-of-component` if you want to run markers for all components.
+You can omit `tests/path-of-component` if you want to run the marker across all components.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| uv run pytest -m <marker> ./tests/path-of-component | |
| ``` | |
| You can omit `./tests/path-of-component` if you want to run markers for all components. | |
| uv run pytest -m <marker> tests/path-of-component | |
| You can omit `tests/path-of-component` if you want to run the marker across all components. |
🤖 Prompt for AI Agents
In docs/GETTING_STARTED.md around lines 67 to 69, the example command uses a
leading './' in the test path which is inconsistent with the repository
conventions. Remove the leading './' from the path so it reads
'tests/path-of-component' to align with the agreed style in the documentation.
Signed-off-by: Milind Waykole <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.