test: Convert L0_simple_go_client to pytest - #8943
Draft
mc-nv wants to merge 10 commits into
Draft
Conversation
Replaces the shell assertions in test.sh with pytest cases so the job reports per-case results instead of a single pass/fail, and so failures name the stage that broke. Layout: Dockerfile thin runtime image over the pipeline's devel image run_tests.py entry point, emits report.xml pytest.ini collection config conftest.py server / stub-generation / client-run fixtures tests/ testcase1-3 Pass/fail semantics are unchanged. The cases assert exactly what the shell version asserted -- 'go run' exits 0, and client.log contains 'Checking Inference Outputs' exactly once -- plus a check that stub generation produced the package, which test.sh left as an unchecked side effect. Server startup remains a hard failure, now via fixture. Notes on the layout: - The entry point is run_tests.py, not pytest.py: a module named pytest.py in this directory shadows the installed pytest package (sys.path[0] is the script directory), which breaks 'import pytest' and 'python -m pytest'. Verified. - pytest.ini sets python_files=testcase*.py; without it pytest collects zero tests from these filenames. Verified. - The Dockerfile deliberately does not COPY the test files. CI mounts the qa-tests volume over /opt/tritonserver/qa, so a COPY to that path is masked at run time. - report.xml is written beside the test, not under logs/, because the log collector copies *.xml out of the test working directory at -maxdepth 1 (ci/templates/utility/template.docker.gitlab-ci.yml). - test.sh is kept as a shim that execs run_tests.py. The shared CI template defaults to TEST_SCRIPT=./test.sh, so this lets the server change and the tritonserver pipeline change land in either order. Every shell variable the old test read is carried over to conftest.py, including TRITON_CLIENT_REPO_TAG. It stays read-but-unused, matching its previous state: the client clone does not pass -b, which is why this test always exercises the remote default branch and cannot see a feature branch. Wiring the tag into the clone is a separate fix. Verified with pytest 8.3.5 in a container: 3 tests collected, fixture graph resolves via --setup-plan, report.xml written, py_compile clean, 'bash -n test.sh' clean, and black 23.1.0 / isort 5.12.0 / flake8 7.3.0 all pass with this repo's configured arguments. The test itself was not executed -- it needs the devel image, a GPU and the model repository.
The CI template passes these through unconditionally, e.g.
-e TRITON_COMMON_REPO_TAG="${TRITON_COMMON_REPO_TAG}"
so an unset pipeline variable reaches the container as an empty string,
not as an absent one. os.environ.get(name, default) treats that as set
and returns '', which the shell form ${VAR:="main"} did not -- it
substitutes on unset or empty.
Consequences of the empty value: 'git clone -b ""' fails the stub
fixture, and int('') raises ValueError while conftest.py is imported,
which aborts collection before any test runs.
Adds _env(), used for TRITON_REPO_ORGANIZATION, TRITON_COMMON_REPO_TAG,
TRITON_CLIENT_REPO_TAG, TRITONSERVER_IPADDR and SERVER_TIMEOUT.
Verified in a container: with all five set to the empty string the
defaults apply and collection succeeds; with values set they are
honoured; black / isort / flake8 clean.
Supersedes the note in 42e524f that said the Dockerfile deliberately does not COPY the test files. It now copies them and carries defaults for the repo tags, so 'docker build && docker run' works on a workstation without mounting anything. CI behaviour is unchanged: the qa-tests volume is mounted over /opt/tritonserver/qa, so the mounted copy shadows the baked-in one, and -e beats image ENV for the tag variables. The three tag ARGs are re-declared after FROM. An ARG given before FROM is in scope only for the FROM line, so the ENV lines would otherwise bake in empty strings -- verified: with the ARGs declared only before FROM, the built image reported an empty value even when --build-arg was passed. BASE_IMAGE keeps a default so a standalone build needs no arguments; CI always overrides it with the devel image it just built. Verified by building and running the image (BASE_IMAGE swapped for python:3.12-slim to keep it small): - defaults -> main / main / main, files present, pytest 8.3.5 - --build-arg -> r26.08 reaches ENV - docker run -e -> overrides image ENV, the CI case - -e VAR= (empty) -> conftest falls back to main - pytest -> 3 tests collected inside the image Note the model repository is not in the build context, so a standalone run still needs ./models supplied.
The test no longer depends on anything Dockerfile.QA stages for it. The Dockerfile now installs the Go toolchain, protoc and both protoc-gen-go plugins itself, on top of a plain Triton runtime image. Previously it added only pytest and relied on Dockerfile.QA:370-381 for Go and the plugins. Note gen_go_stubs.sh needs protoc-gen-go as well as protoc-gen-go-grpc; the test itself only installs the latter, so the former has to come from the image. conftest.py provisions its own model repository by cloning the server repo at TRITON_SERVER_BRANCH_NAME and copying docs/examples/model_repository/ simple, instead of using the models/ directory Dockerfile.QA:87-88 stages. The example model cannot be COPYed in because it lives outside this directory, which is the Docker build context. Also fixes a real defect this verification exposed: _wait_for_server_ready polled without sleeping. While nothing is listening yet urlopen fails on connection refused immediately, so all SERVER_TIMEOUT iterations were consumed in a fraction of a second and the effective wait was ~0 rather than 120s. The first end-to-end run failed with 'Failed to start tritonserver' while the server was still initialising the ONNX backend. util.sh:69-86 sleeps 1s per iteration; this now does the same, in the same order (liveness check, sleep, poll). Verified end to end against gitlab-master.nvidia.com:5005/dl/dgx/tritonserver:26.08-py3-stage (amd64 under emulation, no GPU, no models/ staged, no helper base image): go version go1.25.14 linux/amd64 libprotoc 3.21.12 protoc-gen-go, protoc-gen-go-grpc tests/testcase1.py::test_stubs_generated PASSED [ 33%] tests/testcase2.py::test_go_client_exits_zero PASSED [ 66%] tests/testcase3.py::test_inference_marker_present_once PASSED [100%] ============================ 3 passed in 23.44s ============================ The run exercises the client code on the remote default branch, since the clone still does not pass -b; it validates the harness, not the pending grpc.NewClient change. black / isort / flake8 clean.
The Dockerfile pinned pytest==8.3.5, which downgraded the 9.1.1 already present in the Triton image. Drop the pin and the PYTEST_VERSION arg so the test runs against whatever the image provides. pytest is now the one thing this Dockerfile does not install, which makes it a dependency on the base image. 'RUN python3 -m pytest --version' asserts it at build time so a base image without pytest fails the build with a clear message rather than failing the test run with a ModuleNotFoundError. Verified end to end against gitlab-master.nvidia.com:5005/dl/dgx/tritonserver:26.08-py3-stage (amd64 under emulation, no GPU): platform linux -- Python 3.12.3, pytest-9.1.1, pluggy-1.6.0 tests/testcase1.py::test_stubs_generated PASSED [ 33%] tests/testcase2.py::test_go_client_exits_zero PASSED [ 66%] tests/testcase3.py::test_inference_marker_present_once PASSED [100%] ============================ 3 passed in 21.70s ============================ pytest 9 is a major version ahead of the pin, so this was re-run rather than assumed: collection under python_files=testcase*.py, the rootdir conftest.py import and the JUnit report all behave as they did on 8.3.5.
…go_client
Two gaps against the shell test this replaced.
Verbosity. The shell version ran under 'bash -ex', so every clone, go
install, gen_go_stubs.sh invocation and the client's own output appeared in
the job log. pytest captures fixture output and replays it only on failure,
so a passing run showed almost nothing. pytest.ini now sets -s to keep
sub-actions streaming, plus -ra; _run() echoes each command with its cwd and
its exit status; and the client log is echoed after the run, since
client.log lives four levels deep and the CI collector only reaches
-maxdepth 2, so it was never collected as an artifact -- before this change
either.
Hang diagnostics. run_server() in qa/common/util.sh calls gdb_helper() when
the readiness poll times out, dumping 'thread apply all bt' for a server
that started but never became ready. The conversion dropped that and only
printed the server log. _gdb_backtrace() restores the 'Server Hang' half and
writes gdb_bt.<pid>.log beside the test so the collector picks it up. The
core-dump half is deliberately omitted: the CI template runs the container
with --ulimit core=0 (ci/templates/test/template.test.gitlab-ci.yml:42), so
there are no core files to load. Like gdb_helper, it warns and returns when
gdb is absent -- which is the case in the stage image, so this path is a
no-op there.
Verified end to end against the stage image: 3 passed in 22.41s, with every
sub-action now echoed ('=== Running ...' / '=== Exit 0: ...') and the full
inference output visible in the job log. black / isort / flake8 clean.
GitLab renders ANSI in job logs -- its own step echoes are already coloured -- but the runner gives the job no TTY, so pytest's default --color=auto turned colour off. run_tests.py now passes --color=yes when GITLAB_CI or CI is set, and conftest.py paints its own sub-action banners: cyan for the command, green or red for the exit status. PY_COLORS is honoured first in both places, so PY_COLORS=0 disables colour even in CI and PY_COLORS=1 forces it locally. Outside CI the behaviour is unchanged, so redirecting a local run to a file still yields clean text. Verified in a container: no CI, no tty -> COLOR False, 0 ANSI sequences emitted GITLAB_CI=true -> COLOR True, pytest output carries ESC[32m PY_COLORS=0 + GITLAB_CI -> COLOR False PY_COLORS=1 -> COLOR True black / isort / flake8 clean.
test.sh cloned client.git with no -b, so the test always resolved to the
remote default branch and could never exercise a client feature branch --
even though the CI template has been exporting TRITON_CLIENT_REPO_TAG all
along (ci/templates/test/template.test.gitlab-ci.yml:70). The conversion
carried that over as-is and left the variable read-but-unused.
The clone is now pinned the same way the common.git clone already was:
--single-branch --depth=1 -b ${TRITON_CLIENT_REPO_TAG}. The default is
'main', so an unset variable behaves exactly as before.
This is what lets the job validate a client change. Without it, setting
TRITON_CLIENT_REPO_TAG in a pipeline run had no effect and the test silently
exercised whatever was on the default branch.
Verified in a container with TRITON_CLIENT_REPO_TAG=r26.08:
=== Running git clone --single-branch --depth=1 -b r26.08 .../client.git
=== Exit 0: git
3 passed in 22.43s
The shim existed only because the shared CI template hardcoded
'bash -ex ${TEST_SCRIPT}'. The template now honours TEST_INTERPRETER, and
the job sets python3 / run_tests.py, so the wrapper has no remaining
purpose.
This creates a hard dependency between this commit and the matching
tritonserver change: the two must land together. Until both are in,
whichever side merges first leaves the job red -- server-first asks for a
test.sh that no longer exists, tritonserver-first asks for run_tests.py in
a tree that does not have it yet. The shim previously made either order
safe; that protection is deliberately given up here.
Verified against the real image with test.sh absent, running the CI exec
line verbatim:
/bin/bash -c 'pwd && ls -la && exec ${LOGGER} ${TEST_INTERPRETER:-bash -ex} ${TEST_SCRIPT}'
with TEST_INTERPRETER=python3 TEST_SCRIPT=run_tests.py
-> 3 passed in 22.42s
20 tasks
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| with urllib.request.urlopen(READY_URL, timeout=1) as response: | ||
| if response.status == 200: | ||
| return True | ||
| except (urllib.error.URLError, OSError): |
testcase1/2/3 said nothing about what they assert. They are now testcase1_stub_generation, testcase2_client_exit_status and testcase3_inference_output. The numeric prefix is kept deliberately. Collection order follows filename order, and the first case to run pulls in every session fixture, so its output carries the clone, stub-generation and server-startup logs. Naming them without the prefix sorted client_exit_status first, which left the stub-generation case running last with nothing to show and put the stub output under the wrong heading. Correctness was unaffected either way -- the fixture graph forces the real sequence regardless of file order -- but the job log stopped reading as staged. pytest.ini needs no change: python_files=testcase*.py still matches. Verified with pytest 8.3.5 in a container: 3 tests collected in the order testcase1_stub_generation -> testcase2_client_exit_status -> testcase3_inference_output, and no reference to the old filenames remains anywhere under qa/L0_simple_go_client/.
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.
What does the PR do?
L0_simple_go_clientfrom a shell script to pytest, so the job reports per-case results instead of one pass/fail and failures name the stage that broke.conftest.pyprovisions its own model repository. It no longer relies on anythingDockerfile.QAstages for it.TRITON_CLIENT_REPO_TAG, which the CI template has always exported but the test never used.Pass/fail semantics are unchanged.
Checklist
<commit_type>: <Title>Commit Type:
Check the conventional commit type
box here and add the label to the github PR.
Related PRs:
Where should the reviewer start?
qa/L0_simple_go_client/conftest.py— server lifecycle, stub generation, model-repository provisioning.qa/L0_simple_go_client/Dockerfile— what the test installs for itself.qa/L0_simple_go_client/tests/testcase1-3.py— the three assertions, one per stage.Test plan:
Run by
L0_simple_go_client--base. Verified green in CI (3 passed), and locally againsttritonserver:26.08-py3-stagewith no GPU, no stagedmodels/and no helper image.Three behaviours are checked, matching the shell version exactly: stub generation produced
the package,
go run grpc_simple_client.goexits 0, andclient.logcontainsChecking Inference Outputsexactly once. The first was an unchecked side effect before.Caveats:
run_tests.py, notpytest.py: a module namedpytest.pyin thisdirectory shadows the installed package and breaks both
import pytestandpython -m pytest.pytest.inisetspython_files = testcase*.py; without it pytest collects zero testsfrom these filenames.
a build-time assertion so a base image lacking it fails the build rather than the run.
test.shis gone, which pairs with a matching pipeline change; the two must landtogether or the job is red in between.
Background
Came out of the 26.08 security scan work: the Go client changes in
triton-inference-server/client#919had no way to be exercised, because this test clonedthe client repo without a branch flag and always resolved to the default branch.
Two defects were found by actually running it, neither visible to lint or collection:
_wait_for_server_readypolled without sleeping, so the effective startup timeout was ~0rather than 120s; and repo tags arrived as empty strings rather than unset, which
os.environ.get(name, default)does not treat as absent.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
- Relates to: TRI-1683
CI (internal): [#64992112](http://tritonserver.local/ci/pipelines/64992112)