Skip to content

refactor: Move parameter validation from test methods to fixtures #279

@myakove

Description

@myakove

Summary

Several test methods contain parameter/config validation logic that should be in fixtures instead. Per CLAUDE.md "Validate at Source" principle, validation should happen in fixtures where values originate, not in test methods.

Problem / Motivation

Test methods should focus on testing behavior, not validating configuration. When validation happens in test methods:

  • Tests become harder to maintain
  • Validation logic is duplicated across tests
  • Skip/fail conditions are scattered instead of centralized
  • It violates the "Validate at Source" pattern

Requirements

Violations Found

Location Pattern What's Validated
test_copyoffload_migration.py:209-210 pytest.skip() in test vSphere provider type
test_copyoffload_migration.py:851-852 pytest.fail() in test rdm_lun_uuid config
test_copyoffload_migration.py:1002-1020 pytest.fail() in test storage_vendor_product, datastore_id, secondary_datastore_id
test_copyoffload_migration.py:1785-1796 pytest.fail() in test storage_vendor_product, datastore_id
test_mtv_warm_migration.py:18-27 Module-level skipif Provider type for warm migration

Issues Identified

  1. Redundant validation: copyoffload_config fixture already validates storage_vendor_product and datastore_id, but tests check again
  2. Missing fixture validations: rdm_lun_uuid and secondary_datastore_id should have dedicated fixtures
  3. pytest.skip() and pytest.fail() inside test methods violate the "Validate at Source" pattern

Deliverables

  • Remove redundant validation from test methods (items 3, 4, 5 in table above)
  • Create rdm_lun_uuid validation fixture for RDM tests
  • Create secondary_datastore_id validation fixture for multi-datastore tests
  • Use @pytest.mark.skipif at class level instead of pytest.skip() in test methods
  • Review module-level skipif pattern in warm migration tests
  • Update CLAUDE.md (if codebase structure changes)

Notes

  • The copyoffload_config fixture in tests/conftest.py already validates some fields - avoid duplicating this validation
  • New fixtures should follow the existing pattern: validate at fixture level, raise ValueError with clear message if invalid
  • Use @pytest.mark.skipif with helper functions for conditional test skipping

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions