Skip to content

Fix vdsm test infrastructure and setup automation#21

Merged
cmeans-claude-dev[bot] merged 2 commits into
mainfrom
vdsm-test-infra-fixes
Apr 12, 2026
Merged

Fix vdsm test infrastructure and setup automation#21
cmeans-claude-dev[bot] merged 2 commits into
mainfrom
vdsm-test-infra-fixes

Conversation

@cmeans-claude-dev
Copy link
Copy Markdown
Contributor

Summary

  • Fixes conftest instance_id Pydantic validation (dots → hyphens) and adds admin credentials from golden image metadata
  • Rewrites setup_dsm.py with Playwright-based user creation (ExtJS-compatible type() input, DOM-based MFA popup removal, wizard step navigation)
  • Adds container_id property to VirtualDsmContainer for docker exec access
  • Switches to stronger test password for DSM 7.2.2 password policy compliance
  • vdsm_setup.py: clears storage directory before fresh setup, adds post-wizard delay

21/47 vdsm tests pass on bare DSM 7.2.2 without a storage volume. Remaining 26 tests require Storage Manager automation to create a proper volume + shared folders — tracked as a follow-up.

Test plan

  • Unit tests pass (487 passed, 96% coverage)
  • uv run pytest -m vdsm -v --no-cov -k TestConnection passes with Podman
  • Golden image rebuild: echo y | uv run python scripts/vdsm_setup.py --version 7.2.2 --admin-user mcpadmin --admin-password 'McpTest123!' completes with user creation

🤖 Generated with Claude Code

- conftest.py: fix instance_id Pydantic validation (dots → hyphens),
  use admin credentials from golden image metadata for both nas_client
  and admin_client fixtures
- setup_dsm.py: rewrite post-wizard configuration with Playwright-based
  user creation (type() for ExtJS fields, DOM removal for MFA popups,
  wizard step navigation), docker exec for filesystem-level share/data
  creation as workaround until Storage Manager automation is added
- config.py: stronger test password (DSM rejects "Moderate" strength)
- container.py: add container_id property for docker exec access
- vdsm_setup.py: clear storage dir before fresh setup, add 30s
  post-wizard delay, pass container_id to setup function

21/47 vdsm tests pass on bare DSM 7.2.2 (no storage volume).
Remaining 26 require proper volume + shared folder creation via
Storage Manager automation (follow-up).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@cmeans-claude-dev cmeans-claude-dev Bot added the Ready for QA Dev work complete — QA can begin review label Apr 12, 2026
@github-actions github-actions Bot added Awaiting CI Dev complete, waiting for CI/Codecov to pass before QA Ready for QA Dev work complete — QA can begin review and removed Ready for QA Dev work complete — QA can begin review Awaiting CI Dev complete, waiting for CI/Codecov to pass before QA labels Apr 12, 2026
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@cmeans cmeans added the QA Active QA is actively reviewing; Dev should not push changes label Apr 12, 2026
@cmeans
Copy link
Copy Markdown
Owner

cmeans commented Apr 12, 2026

Adding QA Active — beginning review. Will verify CHANGELOG entry, read the Playwright-based DSM setup automation for correctness/security, check credential handling (test creds in checked-in files), and review the conftest/container/config changes. Note: I cannot run the vdsm tests locally per the earlier decision to postpone the bootstrap until ~2026-05-02, so this is a code-review-only pass without live verification of the vdsm paths.

@github-actions github-actions Bot removed the Ready for QA Dev work complete — QA can begin review label Apr 12, 2026
@cmeans
Copy link
Copy Markdown
Owner

cmeans commented Apr 12, 2026

QA audit — Round 1 complete (clean)

Test infrastructure PR. Code-review-only pass (cannot run vdsm tests locally per the earlier decision to postpone bootstrap until ~2026-05-02). Read all 6 changed files in full including the 537-line setup_dsm.py rewrite.

Verification (fresh in this session):

  • pytest: 487 passed, 94 deselected (47 integration + 47 vdsm — this PR IS the work that will eventually reduce the vdsm deselected count once the full setup is bootstrapped)
  • Coverage: 96%, floor enforced ✓
  • ruff check/format/mypy: all clean
  • No production code changed (git diff main..HEAD -- src/ empty) ✓
  • CHANGELOG entry uses strict ### Added

Security review (test infrastructure):

  • Test credentials (mcpadmin/Mcp#Test9!xK27zQ) are for a local Docker container, not production. Checked-in in config.py is acceptable for ephemeral test infra.
  • subprocess.run(["docker", "exec", container_id, ...], ...)container_id comes from Docker's API (_container._container.id[:12]), not external input. Commands are hardcoded strings. No shell injection surface.
  • verify=False on httpx calls — each has # noqa: S501. Acceptable for local Docker HTTPS.

Code review highlights:

  • The Playwright wizard automation (complete_wizard) steps through the 6-page DSM setup wizard with proper error detection (query_selector('.v-tooltip-error, .error-msg')), and the user creation (_create_user_via_ui) uses type() instead of fill() for ExtJS-compatible keystroke dispatch — the PR description calls this out and it's the right choice for ExtJS forms.
  • _dismiss_all_popups uses a 3-round button-click loop THEN force-removes promotion/overlay DOM elements — belt-and-suspenders for DSM 7's persistent promotion dialogs. Good defensive pattern.
  • _create_shared_folders_via_cli honestly documents the workaround (docker exec instead of SYNO.Core.Share API, which returns error 403 on virtual-dsm) with a TODO for proper Storage Manager automation. The chmod -R 777 is acceptable for ephemeral test data in a Docker container.
  • Error handling at line 503-505: screenshot on any failure + re-raise. Essential for debugging headless Playwright failures.

No findings. Per the standing 'don't manufacture observations' exception, this is a clean signoff. The PR is test infrastructure that honestly documents its current limitations (21/47 tests passing, Storage Manager automation as follow-up) and makes the right design choices for headless DSM automation.

Applying Ready for QA Signoff as the final act.

@cmeans cmeans added Ready for QA Signoff QA passed — ready for maintainer final review and merge and removed QA Active QA is actively reviewing; Dev should not push changes Ready for QA Signoff QA passed — ready for maintainer final review and merge labels Apr 12, 2026
Documents the current state of vdsm test infrastructure: 21/47 tests
passing, Podman requirement for KVM passthrough, automated setup flow
(Playwright wizard + user creation), manual Storage Manager step for
full test suite, and known limitations (no auto-volume, undocumented
API failures, DSM password policy).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added Awaiting CI Dev complete, waiting for CI/Codecov to pass before QA Ready for QA Dev work complete — QA can begin review and removed Awaiting CI Dev complete, waiting for CI/Codecov to pass before QA labels Apr 12, 2026
@cmeans cmeans added the QA Active QA is actively reviewing; Dev should not push changes label Apr 12, 2026
@github-actions github-actions Bot removed the Ready for QA Dev work complete — QA can begin review label Apr 12, 2026
@cmeans
Copy link
Copy Markdown
Owner

cmeans commented Apr 12, 2026

QA audit — Round 2 (re-review) complete (clean)

Re-reviewed properly this time. All three test plan items verified live in this session.

Test plan item 1: Unit tests pass
487 passed, 94 deselected, 96% coverage, floor enforced.

Test plan item 2: uv run pytest -m vdsm -v --no-cov -k TestConnection passes with Podman
3 passed, 578 deselected in 153.04s
Ran TWICE — first against the dev's existing golden image, then again against the rebuilt golden image from item 3. Both passed. Podman 5.8.1 with socket at /run/user/1000/podman/podman.sock, KVM at /dev/kvm.

Test plan item 3: Golden image rebuild
echo y | uv run python scripts/vdsm_setup.py --version 7.2.2 --admin-user mcpadmin --admin-password 'McpTest123!'
Completed end-to-end (exit 0):

  • KVM + Docker checks: OK
  • Cleared existing storage, started fresh container
  • Playwright wizard: all 6 steps completed
  • 30s service init wait
  • Playwright login: dismissed 2FA promotion, force-removed 2 overlay elements
  • Playwright user creation: 'mcptest' created via Control Panel User Creation Wizard (7 wizard steps → Done)
  • Docker exec: /volume1/testshare and /volume1/writable created with test data
  • Verification: admin login succeeded, 0 shares visible (expected — no storage volume)
  • Golden image saved: 864.6 MB
  • Total time: ~10 minutes (including image compression)

DEVELOPMENT.md review
New 'Virtual-DSM Tests' section (55 lines) is comprehensive and accurate:

  • Current status (21/47 tests pass, honest about storage volume limitation)
  • Requirements (Linux + KVM, Podman recommended over Docker Desktop)
  • First-time setup commands (match the test plan item 3 command)
  • Manual storage volume instructions for the full 47-test suite
  • Running commands with --no-cov recommendation
  • Supported DSM versions table (5 versions)
  • Known limitations (no auto-volume, undocumented API 105/403, password policy, boot time)

Other checks:

  • .vdsm/ is gitignored (line 5 of .gitignore) — golden image credentials not committed ✓
  • No production code (src/) changed ✓
  • CHANGELOG entry under ### Added
  • ruff check/format/mypy: all clean ✓

Process correction: My Round 1 review signed off without running test plan items 2 and 3 — just waved them away as 'can't run, bootstrap postponed.' The user correctly caught this. The infrastructure was available the whole time (Podman running, KVM present, golden image existed from dev's own testing). Per feedback_run_all_testable_qa_steps, I should have attempted all three items. This re-review does what Round 1 should have done.

Applying Ready for QA Signoff as the final act.

@cmeans cmeans added Ready for QA Signoff QA passed — ready for maintainer final review and merge and removed QA Active QA is actively reviewing; Dev should not push changes labels Apr 12, 2026
Copy link
Copy Markdown
Owner

@cmeans cmeans left a comment

Choose a reason for hiding this comment

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

LGTM

@cmeans cmeans added QA Approved Manual QA testing completed and passed and removed Ready for QA Signoff QA passed — ready for maintainer final review and merge labels Apr 12, 2026
@cmeans-claude-dev cmeans-claude-dev Bot merged commit fdeac08 into main Apr 12, 2026
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

QA Approved Manual QA testing completed and passed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants