feat(e2e): Add files external provider tests

[matthewgrossman]

Summary

Implements AIRCORE-747: E2E tests for NGC-backed filesets with CI token, plus Hugging Face storage backend tests.

E2E test suite (e2e/files/)

• Reorganized e2e/test_files.py → e2e/files/test_files.py (file upload/download tests).
• New e2e/files/test_storage_backends.py with tests for NGC and Hugging Face external storage backends:
  • TestNGCFileset — creates a secret + NGC-backed fileset pointing at a public resource (nvidia/nemo-microservices/nemo-microservices-quickstart), validates list/download/cache-status. Parametrized error tests verify that invalid key, wrong org, nonexistent resource, and missing secret all return 400 BadRequestError with descriptive messages. Skipped when NGC_API_KEY is not set.
  • TestHuggingFaceFileset — creates an HF-backed fileset (hf-internal-testing/tiny-random-bert), validates list/download/cache-status. Uses HF_TOKEN for authenticated access when available. Error test for nonexistent repo.

CI wiring (.github/workflows/ci.yaml)

• Passes NGC_API_KEY (from NGC_REGISTRY_READ_TOKEN secret) and HF_TOKEN to the python-e2e-test job.

Bug fix: NGC backend error handling (services/core/files/.../ngc.py)

• _get_client(): Now catches NgcException (in addition to ValueError) during client.configure(). Previously, org mismatches raised NgcException which bubbled up as an unhandled 500.
• validate_storage(): Moved _get_registry_api() and _get_target() inside the try block. Previously these calls were outside error handling, so failures like AccessDeniedException from version resolution were unhandled 500s.

E2E infrastructure improvements (e2e/conftest.py)

• Services log on failure: Added a pytest_runtest_makereport hook that appends the tail of the services log to the test report when a test fails. Previously, server-side errors only showed as opaque "500" from the client.
• Per-session log paths: services_log_path is now a session-scoped fixture with a UUID suffix, so parallel pytest processes never clobber each other's logs.

Test plan

• All 11 storage backend tests pass locally (pytest e2e/files/test_storage_backends.py --run-e2e)
• All 5 existing file tests pass after move (pytest e2e/files/test_files.py --run-e2e)
• All 29 NGC backend unit tests pass (pytest services/core/files/tests/test_ngc_backend.py)
• CI python-e2e-test job passes (NGC tests will skip until NGC_REGISTRY_READ_TOKEN secret is created)
• Create NGC_API_KEY repository secret (mapped from NGC_REGISTRY_READ_TOKEN)

Signed-off-by: Matthew Grossman mgrossman@nvidia.com

Summary by CodeRabbit

• Tests

  • Added end-to-end tests for NGC and Hugging Face storage backends covering listing, download, cacheability, and error cases; improved diagnostics to attach recent service logs on failures.

• Bug Fixes

  • Improved and standardized error handling for NGC storage operations.

• Chores

  • CI updated to provide credentials for external storage tests and to collect/upload per-test service log directories.

[github-actions]

Suite	Lines Covered	Line Rate	Branch Rate
Unit Tests	18844/24899	75.7%	62.2%
Integration Tests	12026/23587	51.0%	26.2%

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 09921fbb-c187-4c5b-a915-39da35ca4e55

📥 Commits

Reviewing files that changed from the base of the PR and between 63a6b89 and ab229a0.

📒 Files selected for processing (3)

• .github/workflows/ci.yaml
• e2e/conftest.py
• e2e/files/test_storage_backends.py

🚧 Files skipped from review as they are similar to previous changes (2)

• e2e/files/test_storage_backends.py
• e2e/conftest.py

---

📝 Walkthrough

Walkthrough

Introduces E2E test coverage for NGC and Hugging Face storage backends. Enhances the test harness with per-session log isolation and automatic failure-triggered log tail capture. Strengthens NGC backend error handling to catch SDK exceptions during client initialization and storage validation. Wires provider credentials into CI.

Changes

External Storage Backend E2E Testing

Layer / File(s)	Summary
Test harness improvements: per-session log isolation and failure capture e2e/conftest.py	Replace global _SERVICES_LOG with per-session UUID-suffixed services_log_path fixture; add pytest_runtest_makereport hook to append service log tail to failed test reports; update _services fixture to use the per-session log path.
NGC backend error handling: client creation and storage validation services/core/files/src/nmp/core/files/app/backends/ngc.py	Import NgcException; expand _get_client to catch both ValueError and NgcException, wrapping both in NGCBackendError; update validate_storage error handling to re-raise NGCBackendError directly, map ResourceNotFoundException to NGCBackendError without version suffix, and wrap all other exceptions with raw exception included.
CI environment: NGC and Hugging Face credential secrets and logs .github/workflows/ci.yaml	Add NGC_API_KEY and HF_TOKEN from GitHub secrets to the python-e2e-test step environment; switch services log dir to E2E_SERVICES_LOG_DIR; dump all *.log files from that directory; upload e2e-services-logs/ artifact.
E2E storage backend tests: NGC and Hugging Face filesets e2e/files/test_storage_backends.py	New test module with fixtures for credential lookup, secret creation, and fileset setup. NGC tests verify list/download/cache-status with parameterized error cases (invalid API key, wrong org/team/resource, missing secret); conditionally skip when NGC_API_KEY unset. Hugging Face tests verify list/download/cache-status and nonexistent-repo error; conditionally skip when HF_TOKEN unset.

Possibly related PRs

• NVIDIA-NeMo/nemo-platform#125: Introduced the initial E2E pytest fixtures (_services, sdk, workspace) that this PR extends with log-path isolation and failure-tail capture.
• NVIDIA-NeMo/nemo-platform#179: Also modifies the _services session fixture and e2e startup args/config; relevant to the harness changes here.

Suggested reviewers

• mckornfield
• mikeknep
• svvarom

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	Title clearly summarizes the main change: adding E2E tests for external storage providers (NGC and Hugging Face backends).
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch mgrossman/aircore-747-e2e-tests-ngc-backed-fileset-with-ci-token

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@e2e/conftest.py`:
- Around line 73-75: The current logic always appends a UUID to the services log
filename (base -> path) which breaks CI because when E2E_SERVICES_LOG is set the
CI expects the original unsuffixed `${{ runner.temp }}/services.log`; change the
code so that if E2E_SERVICES_LOG is present in the environment you use that
value unchanged for request.session.stash[_services_log_key], and only generate
a UUID-suffixed path when you fall back to the default tempfile path; adjust the
logic around base and path creation to branch on
os.environ.get("E2E_SERVICES_LOG") and store the appropriate value.

In `@e2e/files/test_storage_backends.py`:
- Around line 197-199: The test's expected error message is outdated; update the
assertion for the case with id "nonexistent-resource" to match
validate_storage's current not-found branch (which produces "NGC resource not
found: ..."). Locate the test vector containing {"target":
"this-resource-does-not-exist-12345"} and replace the existing match string
"Failed to access NGC resource this-resource-does-not-exist-12345" with the new
message produced by validate_storage (e.g., "NGC resource not found:
this-resource-does-not-exist-12345") or change the assertion to a stable
substring/regex that looks for "NGC resource not found" plus the target; ensure
the test still references the id "nonexistent-resource".

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 780c483f-e071-496f-9e78-4b943f339426

📥 Commits

Reviewing files that changed from the base of the PR and between 99a1a25 and 63a6b89.

📒 Files selected for processing (5)

• .github/workflows/ci.yaml
• e2e/conftest.py
• e2e/files/test_files.py
• e2e/files/test_storage_backends.py
• services/core/files/src/nmp/core/files/app/backends/ngc.py

[mikeknep]

LGTM, just the one non-blocking question I'm curious about

[crookedstorm]

Seems good for now. This implies PRs will require copy-pr-bot as soon as we accept contributions. I can test implementing that as soon as we need it. (Folks have told me to wait for something).