test(auditor): E2E tests for Auditor#794
Merged
Merged
Conversation
parkanzky
force-pushed
the
AIRCORE-831/auditor-e2e-k8s
branch
from
July 20, 2026 19:59
f91aad6 to
5c3b0fb
Compare
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds auditor SDK job operations, runtime setup, and end-to-end coverage for plugin health, configuration and target APIs, CLI workflows, filtering and sorting, and Kubernetes-only audit job execution. ChangesAuditor integration coverage
Sequence Diagram(s)sequenceDiagram
participant Test as E2E test
participant SDK as AuditorPluginResource
participant API as Auditor jobs API
Test->>SDK: submit config and target
SDK->>API: POST /v2/workspaces/{workspace}/jobs/audit
API-->>SDK: job JSON
Test->>SDK: list_jobs or get_job
SDK->>API: GET jobs endpoint
API-->>SDK: job status JSON
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
parkanzky
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
e2e/auditor/test_cli.py (1)
52-76: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMissing post-delete verification.
Unlike
test_cli_config_create_list_delete, this doesn't re-list after delete to confirm the target is actually gone.♻️ Proposed addition
result = run_nemo_local("auditor", "targets", "delete", name, "--workspace", workspace, base_url=base_url) assert_exit_0(result, "CLI delete target") + + result = run_nemo_local("auditor", "targets", "list", "--workspace", workspace, base_url=base_url) + assert_exit_0(result, "CLI list targets after delete") + listed = json.loads(result.stdout) + assert all(item["name"] != name for item in listed["data"])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/auditor/test_cli.py` around lines 52 - 76, Extend test_cli_target_create_list_delete after the delete command by listing targets again with the same workspace and base URL, assert the command succeeds, and verify the deleted name is absent from the output.e2e/auditor/test_targets.py (1)
52-69: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winVerify update persisted via a fresh
get().Unlike
test_config_updatein test_configs.py, this only checks theupdate()response, not a re-fetched entity — won't catch an endpoint that echoes the request without persisting.♻️ Proposed addition
assert updated.name == name assert updated.description == "updated" assert updated.model == "gpt-4o" + + retrieved = sdk.auditor.targets.get(workspace=workspace, name=name) + assert retrieved.description == "updated" + assert retrieved.model == "gpt-4o"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/auditor/test_targets.py` around lines 52 - 69, Extend test_target_update to fetch the target again with sdk.auditor.targets.get after update(), then assert the re-fetched entity’s name, description, and model match the updated values. Keep the existing update-response assertions and use the same workspace and name identifiers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@e2e/auditor/test_cli.py`:
- Around line 52-76: Extend test_cli_target_create_list_delete after the delete
command by listing targets again with the same workspace and base URL, assert
the command succeeds, and verify the deleted name is absent from the output.
In `@e2e/auditor/test_targets.py`:
- Around line 52-69: Extend test_target_update to fetch the target again with
sdk.auditor.targets.get after update(), then assert the re-fetched entity’s
name, description, and model match the updated values. Keep the existing
update-response assertions and use the same workspace and name identifiers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d94a210d-d8ca-4c81-af1f-411bf75f574f
📒 Files selected for processing (7)
e2e/auditor/conftest.pye2e/auditor/test_audit_job.pye2e/auditor/test_cli.pye2e/auditor/test_configs.pye2e/auditor/test_healthz.pye2e/auditor/test_targets.pye2e/auditor/utils.py
Contributor
|
parkanzky
force-pushed
the
AIRCORE-831/auditor-e2e-k8s
branch
from
July 20, 2026 21:21
0fe3fdc to
95664b1
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
plugins/nemo-auditor/tests/test_sdk_resources.py (1)
461-500: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAsync
submittest doesn't cover retry-defaults.The sync test (
test_submit_with_string_refs_posts_correct_url_and_body, lines 407-408) assertsmax_probe_retriesandfail_job_on_retries_exhausteddefaults; the async counterpart (line 468-475) only checksconfig/target. Add the same assertions for parity so a regression in the async path isn't missed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/nemo-auditor/tests/test_sdk_resources.py` around lines 461 - 500, Update TestAsyncJobMethods.test_submit_posts_correct_url_and_body to also assert the request body’s default max_probe_retries and fail_job_on_retries_exhausted values, matching the corresponding sync submit test while preserving the existing URL, config, and target assertions.plugins/nemo-auditor/src/nemo_auditor/sdk.py (2)
90-95: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo response-shape validation on new job methods.
plugin_status()validatesisinstance(payload, dict)before returning, butsubmit/list_jobs/get_job(and their async twins) just returnresponse.json()unchecked. If the API ever returns an unexpected shape, callers likee2e/auditor/test_audit_job.py(job_name = job["name"]) get an opaqueKeyError/TypeErrorinstead of a clear SDK-level error.♻️ Example fix for submit()
response.raise_for_status() - return response.json() + payload = response.json() + if not isinstance(payload, dict): + raise TypeError("Auditor job response must be a JSON object.") + return payloadApply the same pattern to
list_jobs,get_job, and their async twins.Also applies to: 106-111, 116-120, 211-216, 227-232, 237-241
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/nemo-auditor/src/nemo_auditor/sdk.py` around lines 90 - 95, Validate the JSON response shape in submit, list_jobs, and get_job, along with their async counterparts, before returning payloads, matching the existing plugin_status() validation pattern. Raise the SDK-level validation error for unexpected response types so callers receive a clear error instead of KeyError or TypeError.
91-91: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated jobs URL path across 6 methods.
f"/v2/workspaces/{ws}/jobs/audit"(and the/audit/{job_name}variant) is repeated in both sync and async classes. Extract a shared helper, e.g._jobs_url(ws, job_name=None), to avoid drift if the path changes.Also applies to: 107-107, 117-117, 212-212, 228-228, 238-238
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/nemo-auditor/src/nemo_auditor/sdk.py` at line 91, Extract the repeated jobs URL construction into a shared `_jobs_url(ws, job_name=None)` helper covering both the collection and `/audit/{job_name}` paths, then update the affected sync and async methods to call it instead of embedding the f-string directly. Ensure all six call sites preserve their existing URL results.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@plugins/nemo-auditor/src/nemo_auditor/sdk.py`:
- Around line 90-95: Validate the JSON response shape in submit, list_jobs, and
get_job, along with their async counterparts, before returning payloads,
matching the existing plugin_status() validation pattern. Raise the SDK-level
validation error for unexpected response types so callers receive a clear error
instead of KeyError or TypeError.
- Line 91: Extract the repeated jobs URL construction into a shared
`_jobs_url(ws, job_name=None)` helper covering both the collection and
`/audit/{job_name}` paths, then update the affected sync and async methods to
call it instead of embedding the f-string directly. Ensure all six call sites
preserve their existing URL results.
In `@plugins/nemo-auditor/tests/test_sdk_resources.py`:
- Around line 461-500: Update
TestAsyncJobMethods.test_submit_posts_correct_url_and_body to also assert the
request body’s default max_probe_retries and fail_job_on_retries_exhausted
values, matching the corresponding sync submit test while preserving the
existing URL, config, and target assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f121702a-a156-428c-b724-d89f1b01f710
📒 Files selected for processing (3)
e2e/auditor/test_audit_job.pyplugins/nemo-auditor/src/nemo_auditor/sdk.pyplugins/nemo-auditor/tests/test_sdk_resources.py
parkanzky
force-pushed
the
AIRCORE-831/auditor-e2e-k8s
branch
from
July 21, 2026 15:25
7cc97ed to
9f3637d
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docker/Dockerfile.auditor-tasks`:
- Around line 67-68: Update the Dockerfile command sequence so uv sync and the
garak installation target the same /app/.garak_venv environment. Run uv sync
before installing garak, or explicitly configure the sync command to use that
virtual environment while preserving the existing frozen project options.
In `@e2e/auditor/test_cli.py`:
- Around line 74-75: Extend the deletion test after the successful
`run_nemo_local` and `assert_exit_0` calls to list targets and assert that
`name` is absent, matching the existing config test’s verification pattern.
- Around line 78-112: Update test_cli_config_update to register cleanup for the
created configuration immediately after the create command succeeds, using the
test fixture finalizer or a finally block. Ensure the cleanup invokes the
existing configuration delete CLI flow for name and workspace, including when
later update or get assertions fail.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 01e06e18-4960-4491-a962-e26fa533f1fc
📒 Files selected for processing (10)
docker/Dockerfile.auditor-taskse2e/auditor/conftest.pye2e/auditor/test_audit_job.pye2e/auditor/test_cli.pye2e/auditor/test_configs.pye2e/auditor/test_healthz.pye2e/auditor/test_targets.pye2e/auditor/utils.pyplugins/nemo-auditor/src/nemo_auditor/sdk.pyplugins/nemo-auditor/tests/test_sdk_resources.py
🚧 Files skipped from review as they are similar to previous changes (7)
- e2e/auditor/utils.py
- e2e/auditor/test_healthz.py
- e2e/auditor/test_configs.py
- e2e/auditor/conftest.py
- plugins/nemo-auditor/src/nemo_auditor/sdk.py
- e2e/auditor/test_targets.py
- e2e/auditor/test_audit_job.py
crookedstorm
approved these changes
Jul 21, 2026
crookedstorm
left a comment
crookedstorm
left a comment
Contributor
There was a problem hiding this comment.
I think this looks good.
Signed-off-by: Paul A. Parkanzky <parkanzky@users.noreply.github.com>
Signed-off-by: Paul A. Parkanzky <parkanzky@users.noreply.github.com>
Signed-off-by: Paul A. Parkanzky <parkanzky@users.noreply.github.com>
Signed-off-by: Paul A. Parkanzky <parkanzky@users.noreply.github.com>
Signed-off-by: Paul A. Parkanzky <parkanzky@users.noreply.github.com>
parkanzky
force-pushed
the
AIRCORE-831/auditor-e2e-k8s
branch
from
July 21, 2026 20:41
7d910fb to
df52296
Compare
Signed-off-by: Paul A. Parkanzky <parkanzky@users.noreply.github.com>
parkanzky
force-pushed
the
AIRCORE-831/auditor-e2e-k8s
branch
from
July 21, 2026 20:42
df52296 to
1a618c2
Compare
soluwalana
pushed a commit
that referenced
this pull request
Jul 22, 2026
* tests(auditor): E2E tests for Auditor Signed-off-by: Paul A. Parkanzky <parkanzky@users.noreply.github.com> * link async submit Signed-off-by: Paul A. Parkanzky <parkanzky@users.noreply.github.com> * re-add garak install to docker file Signed-off-by: Paul A. Parkanzky <parkanzky@users.noreply.github.com> * rm superfluous uv sync Signed-off-by: Paul A. Parkanzky <parkanzky@users.noreply.github.com> * switch back to default cpu/cpu provider/profile Signed-off-by: Paul A. Parkanzky <parkanzky@users.noreply.github.com> * skip tests that depend on future image Signed-off-by: Paul A. Parkanzky <parkanzky@users.noreply.github.com> --------- Signed-off-by: Paul A. Parkanzky <parkanzky@users.noreply.github.com> Co-authored-by: Paul A. Parkanzky <parkanzky@users.noreply.github.com> Signed-off-by: Sam Oluwalana <soluwalana@nvidia.com>
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
2 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.
Auditor E2E tests.
Summary by CodeRabbit
garakversion to0.15.1for more consistent test runs.