refactor: fold duplicated _first_value into shared ssh_config

[mrbobbytables]

Refactor

Context: #763 flags that every behave suite hand-wires its own environment.py with copy-pasted cross-cutting concerns. One concretely named instance: _first_value(*values) is defined identically three times — tests/shared/ssh_config.py:32, tests/common/features/environment.py:23, and tests/kde-smoke/features/environment.py:35.

What this does:

• Removes the two duplicate _first_value definitions from tests/common and tests/kde-smoke; both now import the existing implementation from tests.shared.ssh_config.
• No behaviour change — all three implementations were byte-for-byte identical.
• Extends tests/unit/test_suite_environment_contract.py (the ast-based contract test added by [architect] refactor: enforce behave suite environment quarantine-gate contract (new tests/unit/test_suite_environment_contract.py) #765 for the quarantine gate) with the same pattern for _first_value: a suite environment.py may not define its own copy, and if it calls the helper it must import it from tests.shared.ssh_config. This turns the fix into an enforced contract instead of a one-off cleanup that could silently regress.

Scope discipline (per #763's own recommendation-1 wording: "fold the duplicated _first_value into the existing tests/shared/ssh_config definition"): this PR does only that. It deliberately does not touch:

• Extracting the full shared before_scenario/before_all preamble into a tests/shared/suite_env.py (proposed step 1 in [architect] behave suite environment contract is unenforced convention: 14 hand-wired environment.py, quarantine gate copy-pasted, timing wired in only 10/14 #763) — that is a 14-file, behaviour-sensitive refactor that needs a real E2E run behind it, already called out as future work by [architect] refactor: enforce behave suite environment quarantine-gate contract (new tests/unit/test_suite_environment_contract.py) #765.
• Wiring record_start/record_end timing into the four suites that lack it — blocked on the still-open mandatory-vs-opt-in decision in [architect] tests/shared/timing.py is an orphaned subsystem: timings.jsonl has no consumer, DEFAULT_SLA and SLA_STRICT are never read outside their own tests #764.

Verification:

• pytest tests/unit -q — 1541 passed.
• ruff check on the two edited environment.py files shows the same 4 pre-existing warnings as before the change (verified via git stash) — no new lint issues introduced.

Ref #763.

— hive: backend=copilot model=claude-sonnet-5

🐝 Hive Agent: contributor | SHA: 8f272f09

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 90.90909% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
tests/unit/test_suite_environment_contract.py	89.47%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

[kubestellar-hive]

Approved by @castrojo for Hive auto-merge on green CI.

[hanthor]

Premise still holds — needs a rebase

The duplication this PR removes is still present on main:

$ grep -rn "_first_value" tests/ --include=*.py
tests/shared/ssh_config.py:32:def _first_value(*values: str) -> str:
tests/common/features/environment.py:23:def _first_value(*values: str) -> str:
tests/kde-smoke/features/environment.py:35:def _first_value(*values: str) -> str:

So the change is still worth landing, and extending test_suite_environment_contract.py (which is on main) so the dedup is enforced rather than a one-off cleanup is the right call.

Blocker

mergeable_state: dirty. Confirmed with a local test merge onto main at db329ee:

PR787 CONFLICT

Rebase and re-push. Given the change is byte-identical-function removal plus two imports plus a contract test, the rebase should be mechanical.

After the rebase, please re-verify the lint claim

The description says ruff check on the two edited environment.py files shows "the same 4 pre-existing warnings". On current main the PR-gate command is clean:

$ ruff check tests/ scripts/ --select E,F,W --ignore E501
All checks passed!

So whatever those 4 were, they are outside the E,F,W selection the gate uses — worth re-confirming post-rebase that ruff check tests/ scripts/ --select E,F,W --ignore E501 still reports All checks passed!, since removing a function from a module that also does from ... import * is exactly the shape that produces new F405s. (#791 is currently tripping on precisely that.)

---

Generated by Claude Code