Skip to content

[no-relnote] Add E2E for libnvidia-container #1118

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ArangoGutierrez
Copy link
Collaborator

@ArangoGutierrez ArangoGutierrez commented May 30, 2025

⚠️ PR under refactor ⚠️
This patch adds an E2E test for the nvidia-container-cli that will allow us to catch regressions on libnvidia-container

@ArangoGutierrez ArangoGutierrez requested review from elezar and Copilot May 30, 2025 14:35
@ArangoGutierrez ArangoGutierrez self-assigned this May 30, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds an end-to-end test for the nvidia-container-cli to help catch regressions in libnvidia-container.

  • Introduces a new Go test file under tests/e2e to run E2E validations using Ginkgo and Gomega.
  • Updates the Makefile to focus on the nvidia-container-cli tests by default.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
tests/e2e/nvidia-container-cli_test.go New E2E test implementation for validating the nvidia-container-cli.
tests/e2e/Makefile Updated test target to add a focus flag for running nvidia-container-cli tests.

@coveralls
Copy link

coveralls commented May 30, 2025

Pull Request Test Coverage Report for Build 15472130908

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 33.553%

Totals Coverage Status
Change from base Build 15466637032: 0.0%
Covered Lines: 4272
Relevant Lines: 12732

💛 - Coveralls

@ArangoGutierrez ArangoGutierrez force-pushed the e2e/nvidia-container-cli branch from 67cc2ec to 903737e Compare May 30, 2025 15:32
@ArangoGutierrez ArangoGutierrez requested a review from elezar May 30, 2025 15:32
@ArangoGutierrez ArangoGutierrez force-pushed the e2e/nvidia-container-cli branch 5 times, most recently from d0a338e to 8c42c14 Compare June 3, 2025 18:51
@ArangoGutierrez
Copy link
Collaborator Author

Tests pass, PR ready for review @elezar

@ArangoGutierrez ArangoGutierrez force-pushed the e2e/nvidia-container-cli branch from 8c42c14 to d905a49 Compare June 4, 2025 08:13
Comment on lines 31 to 33
docker run -d --name test-nvidia-container-cli \
--privileged \
--runtime=nvidia \
Copy link
Member

Choose a reason for hiding this comment

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

It is not scalable to have to MOUNT everything into this container. Note that when we still had some simple integration tests in the toolkit we used

testing::docker::dind::setup() {

Can we rather adapt this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sure, will work on that. we need to make this test more robust and scalable

Signed-off-by: Carlos Eduardo Arango Gutierrez <[email protected]>
@ArangoGutierrez ArangoGutierrez force-pushed the e2e/nvidia-container-cli branch from d905a49 to 9674787 Compare June 5, 2025 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants