test: Adjust e2e test for Docker 29.2.0#1615
Conversation
Pull Request Test Coverage Report for Build 21436883306Details
💛 - Coveralls |
|
/cherry-pick release-1.18 |
|
|
||
| It("should not fail when using the nvidia-container-runtime", func(ctx context.Context) { | ||
| _, _, err := runner.Run("docker run --rm --runtime=nvidia -e NVIDIA_VISIBLE_DEVICES=all -e NVIDIA_DRIVER_CAPABILITIES=all firmware-test") | ||
| output, _, err := runner.Run("docker run --rm --runtime=nvidia -e NVIDIA_VISIBLE_DEVICES=all -e NVIDIA_DRIVER_CAPABILITIES=all firmware-test") |
There was a problem hiding this comment.
If I read correctly, this might be silently dropping error detail that may matter at some point to someone :-) We drop stderr of the child process here, right?
"Silently dropping or even changing error detail that may matter at some point to someone" is I think chapter one, sin 1 in my imaginary book about error handling :)
But you probably know better here!
There was a problem hiding this comment.
Yes, we do drop stderr here. I understand that we should be more dilegent about checking these, but that's a bigger change than I want o make in this PR. I would like to revisit our testing strategy and organisation for this repo in general in the near future.
(Let me at least update this PR to explicitly check stderr).
There was a problem hiding this comment.
For the time being I updated the tests to consisder the docker version on the host and check the applicable conditions based on that.
9631b9d to
feb19b6
Compare
Signed-off-by: Evan Lezar <elezar@nvidia.com>
feb19b6 to
dd10e43
Compare
|
🤖 Backport PR created for |
As of Docker 29.2.0, the
--gpus=allflag is converted to a--device=nvidia.com/gpu=allrequest. This means that tests that expect DIFFERENT behaviour when using the--gpus=allflag and other flags are not applicable.This change updates the firmware injection test for the new behaviour.