Skip to content

fix: e2e test harness CI#5275

Open
jalolk wants to merge 3 commits into
developfrom
fix-e2e-test-harness-socks5-proxy
Open

fix: e2e test harness CI#5275
jalolk wants to merge 3 commits into
developfrom
fix-e2e-test-harness-socks5-proxy

Conversation

@jalolk
Copy link
Copy Markdown

@jalolk jalolk commented May 12, 2026

Ticket

JIRA-NYM-XXXX

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.

Checklist:

  • Changelog

Screenshots (optional, if UI related)


This change is Reviewable

Summary by CodeRabbit

  • Chores
    • CI pipeline updated to install a Rust target, set up Zig and cargo-zigbuild, switch to zig-based release builds, and include an additional proxy binary among test artifacts.
  • Tests
    • Added embedded bootstrapping scripts for circumvention tests.
    • Test VM provisioning now writes those scripts directly into guest VMs, removing runtime reliance on host-side files.

Review Change Stack

@jalolk jalolk self-assigned this May 12, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 12, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 62b4b165-6fc8-4da3-b505-ed74ae9e7e34

📥 Commits

Reviewing files that changed from the base of the PR and between 126e31e and fbde405.

📒 Files selected for processing (1)
  • .github/workflows/e2e-test.yml
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/e2e-test.yml

📝 Walkthrough

Walkthrough

Adds the nym-socks5-proxy binary to the e2e build and artifact collection, and changes VM provisioning to embed blocking scripts at compile time and write them into guest VMs via SSH instead of transferring host files.

Changes

E2E Test Pipeline Extension

Layer / File(s) Summary
Add nym-socks5-proxy to e2e test build and distribution
.github/workflows/e2e-test.yml
The workflow installs the x86_64-unknown-linux-gnu Rust target, sets up Zig and cargo-zigbuild, switches to cargo zigbuild --release for that target, includes nym-socks5-proxy in the compiled crates, and copies target-specific release binaries into /tmp/dist/ for artifact upload.

VM provisioning and embedded blocking scripts

Layer / File(s) Summary
Embed blocking scripts as compile-time byte slices
nym-vpn-core/crates/test/test-manager/src/tests/config_nym.rs
Adds IP_BLOCK_SCRIPT, SNI_BLOCK_SCRIPT, and DELAYED_IP_BLOCK_SCRIPT as include_bytes!-embedded &[u8] constants sourced via CARGO_MANIFEST_DIR.
Write embedded scripts to guest via SSH
nym-vpn-core/crates/test/test-manager/src/vm/provision.rs
Imports the new script constants and replaces host-file transfers with writing an in-memory (filename, bytes) list to the guest using ssh_write_with_opts, marking scripts executable.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I stitched the scripts into the build so neat,
No more host-to-guest shoeing of tiny feet.
A proxy hops in the e2e night,
Scripts land by SSH, all snug and tight. 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is entirely a template placeholder with no actual summary of changes, motivation, context, JIRA ticket details, or specific implementation information provided. Fill in the description with a summary of changes (Zig integration, cargo-zigbuild usage, nym-socks5-proxy addition), the issue being fixed, motivation/context, and update the JIRA-NYM-XXXX placeholder with the actual ticket number.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: e2e test harness CI' is somewhat related to the changes, which involve fixing CI for e2e tests, but it lacks specificity about the actual modifications (Zig/zigbuild integration and nym-socks5-proxy additions).
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 unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-e2e-test-harness-socks5-proxy

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@jalolk jalolk force-pushed the fix-e2e-test-harness-socks5-proxy branch 2 times, most recently from d027894 to 126e31e Compare May 13, 2026 12:10
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
.github/workflows/e2e-test.yml (1)

60-61: ⚡ Quick win

Consider optimizing cargo-zigbuild installation.

The cargo install command will reinstall cargo-zigbuild if the cache is missed or invalidated, which can be time-consuming. Consider adding a conditional check to skip installation if the binary already exists.

⚡ Proposed optimization
     - name: Install cargo-zigbuild
-      run: cargo install --locked cargo-zigbuild
+      run: |
+        if ! command -v cargo-zigbuild &> /dev/null; then
+          cargo install --locked cargo-zigbuild
+        fi
🤖 Prompt for 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.

In @.github/workflows/e2e-test.yml around lines 60 - 61, The GitHub Actions step
named "Install cargo-zigbuild" currently always runs `cargo install --locked
cargo-zigbuild`; change it to first check for the presence of the cargo-zigbuild
binary (e.g., use `command -v cargo-zigbuild` or `[ -x "$(command -v
cargo-zigbuild)" ]`) and only run `cargo install --locked cargo-zigbuild` if the
check fails, so the job skips reinstall when the binary already exists or is
restored from cache.
🤖 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.

Nitpick comments:
In @.github/workflows/e2e-test.yml:
- Around line 60-61: The GitHub Actions step named "Install cargo-zigbuild"
currently always runs `cargo install --locked cargo-zigbuild`; change it to
first check for the presence of the cargo-zigbuild binary (e.g., use `command -v
cargo-zigbuild` or `[ -x "$(command -v cargo-zigbuild)" ]`) and only run `cargo
install --locked cargo-zigbuild` if the check fails, so the job skips reinstall
when the binary already exists or is restored from cache.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 39f2308c-abae-48b8-8bd0-e05f71f061f7

📥 Commits

Reviewing files that changed from the base of the PR and between 8d7f389 and 126e31e.

📒 Files selected for processing (1)
  • .github/workflows/e2e-test.yml

@jalolk jalolk force-pushed the fix-e2e-test-harness-socks5-proxy branch from 126e31e to 6716756 Compare May 13, 2026 12:26
@jalolk jalolk force-pushed the fix-e2e-test-harness-socks5-proxy branch from 6716756 to fbde405 Compare May 19, 2026 14:43
@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot

See analysis details on SonarQube Cloud

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.

1 participant