Skip to content

test(evaluation): add unit tests for _eval_set_results_manager_utils#6205

Open
Koushik-Salammagari wants to merge 2 commits into
google:mainfrom
Koushik-Salammagari:test/eval-set-results-manager-utils-coverage
Open

test(evaluation): add unit tests for _eval_set_results_manager_utils#6205
Koushik-Salammagari wants to merge 2 commits into
google:mainfrom
Koushik-Salammagari:test/eval-set-results-manager-utils-coverage

Conversation

@Koushik-Salammagari

Copy link
Copy Markdown
Contributor

Link to Issue or Description of Change

Description of the change (no existing issue):

Problem:
The google.adk.evaluation._eval_set_results_manager_utils module had no
dedicated unit-test coverage. It contains non-trivial logic that is easy to
regress, most notably parse_eval_set_result_json, which has a
backward-compatibility fallback for legacy double-encoded result files (where
the outer JSON is a string containing the inner JSON object).

Solution:
Add a focused unit-test module that covers all three functions and their edge
cases. No production code is changed — this is a test-only PR that locks in the
current behavior.

Coverage added:

  • _sanitize_eval_set_result_name — slash replacement, multiple slashes, and
    no-op cases.
  • create_eval_set_result{app_name}_{eval_set_id}_{timestamp} id
    encoding, that the timestamp matches creation_timestamp, name sanitization,
    and empty eval_case_results.
  • parse_eval_set_result_json — standard JSON string, bytes input,
    camelCase-aliased JSON, the legacy double-encoded format, and error paths for
    a JSON object missing required fields and for non-JSON input.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.
$ pytest tests/unittests/evaluation/test__eval_set_results_manager_utils.py -v
...
14 passed in 1.08s

Manual End-to-End (E2E) Tests:

Not applicable — this is a test-only change with no runtime/user-facing impact.

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

Adds unit-test coverage for the previously untested
_eval_set_results_manager_utils module, covering:

- _sanitize_eval_set_result_name: slash replacement and no-op cases.
- create_eval_set_result: id/timestamp encoding and name sanitization.
- parse_eval_set_result_json: standard, bytes, camelCase-aliased, and
  legacy double-encoded JSON, plus error paths for malformed input.
@adk-bot adk-bot added the eval [Component] This issue is related to evaluation label Jun 23, 2026
@rohityan rohityan self-assigned this Jun 24, 2026
@rohityan rohityan added the needs review [Status] The PR/issue is awaiting review from the maintainer label Jun 26, 2026
@rohityan rohityan requested a review from DeanChensj June 26, 2026 03:17
@rohityan

Copy link
Copy Markdown
Collaborator

Hi @Koushik-Salammagari , Thank you for your contribution! We appreciate you taking the time to submit this pull request. Your PR has been received by the team and is currently under review. We will provide feedback as soon as we have an update to share.

@rohityan

Copy link
Copy Markdown
Collaborator

Hi @DeanChensj , can you please review this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

eval [Component] This issue is related to evaluation needs review [Status] The PR/issue is awaiting review from the maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants