Skip to content

refactor(server): deduplicate test helpers and grpc utilities#1708

Merged
johntmyers merged 1 commit into
NVIDIA:mainfrom
ericcurtin:refactor/deduplicate-server-helpers
Jun 3, 2026
Merged

refactor(server): deduplicate test helpers and grpc utilities#1708
johntmyers merged 1 commit into
NVIDIA:mainfrom
ericcurtin:refactor/deduplicate-server-helpers

Conversation

@ericcurtin
Copy link
Copy Markdown
Contributor

Summary

Remove three groups of copy-pasted code from `openshell-server` with no behaviour change.

Related Issue

N/A — housekeeping.

Changes

  • Remove duplicate `current_time_ms()` wrapper in `grpc/mod.rs`. The same
    one-liner was already exported from `persistence/mod.rs`. Update the three
    grpc sub-modules (`policy`, `sandbox`, `service`) to import directly from
    `crate::persistence` and drop the inline local use inside
    `handle_create_sandbox_inner`.

  • Consolidate `test_store()` — verbatim in seven `#[cfg(test)]` blocks.
    Promote a single canonical version to `persistence/mod.rs` (cfg-gated) and
    replace all copies. `supervisor_session` retains a thin `Arc` wrapper
    that calls through to the shared helper.

  • Move `grpc_client_mtls()` and `build_tls_root()` into the existing
    `tests/common/mod.rs` shared module; remove the duplicates from
    `edge_tunnel_auth.rs` and `multiplex_tls_integration.rs`. Clean up the
    now-unused `RootCertStore` and `Channel` imports those files had.

Net: 14 files, -54 lines.

Testing

  • `mise run pre-commit` passes (rust fmt, clippy -D warnings)
  • `cargo test -p openshell-server --lib` — 761 tests, 0 failed
  • E2E tests added/updated (not applicable — no behaviour change)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)

@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented Jun 3, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@TaylorMutch TaylorMutch self-assigned this Jun 3, 2026
@TaylorMutch
Copy link
Copy Markdown
Collaborator

/ok to test e98a25a

@johntmyers johntmyers added the gator:blocked Gator is blocked by process or repository gates label Jun 3, 2026
@johntmyers
Copy link
Copy Markdown
Collaborator

johntmyers commented Jun 3, 2026

gator-agent

Maintainer Approval Needed

Gator validation and PR monitoring are complete.

Validation: small, concentrated openshell-server helper/test deduplication with no user-facing behavior change.
Review: code-only review completed; no blocking findings remain.
Checks: required statuses are green for head 86c288bfb227e135637661d3134e6e1c7145eca2 (OpenShell / Branch Checks, OpenShell / Helm Lint, OpenShell / E2E, and OpenShell / GPU E2E).
E2E: N/A; no test:* labels needed for this behavior-preserving refactor.

Human maintainer approval or merge decision is now required.

Remove three groups of copy-pasted code in openshell-server:

1. grpc/mod.rs had a private current_time_ms() wrapper identical to the
   one already exported from persistence/mod.rs. Remove the duplicate
   and update the three grpc sub-modules (policy, sandbox, service) to
   import directly from crate::persistence.

2. test_store() was repeated verbatim in seven #[cfg(test)] blocks.
   Promote a single canonical version to persistence/mod.rs (cfg-gated)
   and replace all copies with crate::persistence::test_store() calls or
   a thin Arc wrapper in supervisor_session.

3. grpc_client_mtls() and build_tls_root() were copy-pasted across
   edge_tunnel_auth.rs and multiplex_tls_integration.rs. Move both into
   the existing tests/common/mod.rs shared module and import from there.
@ericcurtin ericcurtin force-pushed the refactor/deduplicate-server-helpers branch from e98a25a to 86c288b Compare June 3, 2026 16:34
@johntmyers
Copy link
Copy Markdown
Collaborator

/ok to test 86c288b

@johntmyers johntmyers added gator:watch-pipeline Gator is monitoring PR CI/CD status gator:approval-needed Gator completed review; maintainer approval needed and removed gator:blocked Gator is blocked by process or repository gates gator:watch-pipeline Gator is monitoring PR CI/CD status labels Jun 3, 2026
@johntmyers johntmyers merged commit d5b79e5 into NVIDIA:main Jun 3, 2026
31 of 33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gator:approval-needed Gator completed review; maintainer approval needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants