Skip to content

Conversation

@kthui
Copy link
Contributor

@kthui kthui commented Jan 24, 2025

What does the PR do?

Add tests for Python backend response parameters.

Checklist

  • PR title reflects the change and is of format <commit_type>: <Title>
  • Changes are described in the pull request.
  • Related issues are referenced.
  • Populated github labels field
  • Added test plan and verified test passes.
  • Verified that the PR passes existing CI.
  • Verified copyright is correct on all changed files.
  • Added succinct git squash message before merging ref.
  • All template sections are filled out.
  • Optional: Additional screenshots for behavior/output changes with before/after.

Commit Type:

Check the conventional commit type
box here and add the label to the github PR.

  • build
  • ci
  • docs
  • feat
  • fix
  • perf
  • refactor
  • revert
  • style
  • test

Related PRs:

triton-inference-server/python_backend#394

Where should the reviewer start?

Start with test_setting_three_element_response_parameters test case first to get a sense on the feature, and then move on to other test cases, and finish with test_setting_response_parameters_decoupled test case.

Test plan:

N/A

  • CI Pipeline ID: (See the related Python backend PR for the pipeline Id)

Caveats:

N/A

Background

N/A

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

N/A

@kthui kthui added the PR: test Adding missing tests or correcting existing test label Jan 24, 2025
fi

set +e
python3 -m pytest --junitxml=response_parameters_test.report.xml response_parameters_test.py > $TEST_LOG 2>&1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for using pytest to generate the report 🚀

* Testing to check parameters set in response object

* Complete basic response parameter test case

* Complete the response parameters test and add decoupled test

* Add leak detector to tests

* Pre-commit fix

* Add non-str key type to test case
@kthui kthui force-pushed the jacky-py-res-param branch from adb5931 to 9370d22 Compare January 24, 2025 19:25
Copy link
Contributor

@krishung5 krishung5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, really nice detailed test cases!

@kthui kthui merged commit 441d7cf into main Jan 25, 2025
3 checks passed
@kthui kthui deleted the jacky-py-res-param branch January 25, 2025 00:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: test Adding missing tests or correcting existing test

Development

Successfully merging this pull request may close these issues.

4 participants