Skip to content

test(synapse): use neutral fixture hostname in mutation-exec tests - #326

Open
jmagar wants to merge 2 commits into
mainfrom
fix/mutation-exec-devhost-fixture
Open

test(synapse): use neutral fixture hostname in mutation-exec tests#326
jmagar wants to merge 2 commits into
mainfrom
fix/mutation-exec-devhost-fixture

Conversation

@jmagar

@jmagar jmagar commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • PR feat(infra): add bounded execution mutations #317 was merged before a review finding on its new host/container exec test fixtures was addressed: they used a literal internal hostname as a placeholder value.
  • Renames that fixture identifier to a neutral placeholder consistently across the four new test files it appears in.

Test plan

  • cargo test -p synapse-application mutation_exec (10 passed)
  • cargo test -p synapse-application (full crate suite, all passed)
  • cargo test -p soma-infra --features process-driver targeted at the touched process_host_exec tests (3 passed)

Copilot AI lite review requested due to automatic review settings August 5, 2026 05:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@jmagar

jmagar commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator Author

Reviewed the outstanding finding on this PR: the note that other fleet/infra test suites (crates/shared/operations/fleet/src/*_tests.rs, crates/shared/operations/infra/src/*_tests.rs) still use a real hostname/username as fixture placeholders.

That's correctly flagged as pre-existing and out of scope here — this PR only touches the four synapse mutation-exec test files it set out to fix. Left those other files untouched to keep this change path-limited, and opened a follow-up task to rename those fixture literals to neutral placeholders (e.g. devhost/devuser) for consistency with this PR's precedent.

No other findings on this PR; CI is green and mergeable.

@jmagar

jmagar commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator Author

Reviewed the automated findings for this PR.

The only item raised is informational and explicitly out of scope: it notes that other fixture test suites (crates/shared/operations/fleet/src/*_tests.rs, crates/shared/operations/infra/src/*_tests.rs) still hardcode a real internal hostname/username as placeholder values, and recommends a follow-up PR to apply the same neutral-placeholder treatment this PR already does for the mutation-exec fixtures. That work touches files outside this PR's diff, so it's intentionally left for a separate PR rather than folded in here.

No changes were needed in this PR itself:

  • Re-verified cargo test -p synapse-application mutation_exec on the current branch tip: 10 passed, 0 failed.
  • All CI checks on this PR are currently green.

Flagged the fleet/infra fixture cleanup as a follow-up task so it isn't lost.

jmagar added a commit that referenced this pull request Aug 5, 2026
…333)

Replace the real username 'jmagar' with the neutral placeholder
'devuser' in fleet and infra test fixtures, including the
/home/jmagar/.ssh/* paths in the OpenSSH connector and endpoint
tests and the user column of the ps-output fixtures.

Follows the precedent set by #326 (devhost fixture hostname) and
completes the hostname scrub in #325, which neutralized host names
but left the username in place.

Test fixture literals only; no behavior change.
PR #317 merged before its review finding on a literal internal
hostname in the new host/container exec test fixtures was addressed.
Replace it with a neutral placeholder now that it landed on main.
@jmagar
jmagar force-pushed the fix/mutation-exec-devhost-fixture branch from d676f17 to 60989bb Compare August 6, 2026 01:33
Copilot AI review requested due to automatic review settings August 6, 2026 01:33

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

jmagar added a commit that referenced this pull request Aug 6, 2026
* chore: scrub internal network identifiers

Replace internal IP addresses, the internal tailnet domain, and host
aliases in tracked files with neutral documentation placeholders
(192.0.2.0/24, 198.51.100.0/24, example.ts.net, *.example.internal,
role-based host aliases).

* fix: address review findings on identifier scrub

- Run cargo fmt on topology_tests.rs (line exceeded wrap width
  after the devhost rename)
- Rename old_dookie/new_dookie -> old_devhost/new_devhost in
  cache_tests.rs so local variable names don't retain the raw
  internal hostname substring
- Rename squirts_pos -> edgehost_pos in formatters_tests.rs for
  the same reason
- Re-pad the doc-comment example markdown table in
  formatters/host.rs after the squirts->edgehost substitution
  widened a column

* test(fleet,infra): use neutral fixture username in operations tests (#333)

Replace the real username 'jmagar' with the neutral placeholder
'devuser' in fleet and infra test fixtures, including the
/home/jmagar/.ssh/* paths in the OpenSSH connector and endpoint
tests and the user column of the ps-output fixtures.

Follows the precedent set by #326 (devhost fixture hostname) and
completes the hostname scrub in #325, which neutralized host names
but left the username in place.

Test fixture literals only; no behavior change.
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.

2 participants