refactor(tests): standardize tests to follow pytest conventions

[agsaru]

PR Type

• Bug fix
• New feature
• Core Runtime change (higher bar -- see CONTRIBUTING.md)
• Docs / tooling
• Refactoring

Summary

This PR refactors existing tests to align with the project's testing guidelines. Using pytest inplace of unittest. Replacing unittest.mock with pytest-mock

[codecov]

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (master@732ca76). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
metaflow/plugins/pypi/pip.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #3242   +/-   ##
=========================================
  Coverage          ?   28.86%           
=========================================
  Files             ?      381           
  Lines             ?    52467           
  Branches          ?     9260           
=========================================
  Hits              ?    15145           
  Misses            ?    36313           
  Partials          ?     1009           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR updates how a pip index-related regression test mocks Pip._call, and fixes a regex string literal to avoid escape-sequence issues when extracting package names from pip error output.

Changes:

• Replace monkeypatch/MagicMock usage with pytest-mock’s mocker.patch.object in the pip indices test.
• Use a raw string for the \w* regex in pip.py to prevent invalid escape-sequence behavior.

Reviewed changes

Copilot reviewed 64 out of 64 changed files in this pull request and generated 3 comments.

File	Description
test/plugins/pip/test_pip_indices.py	Switches test mocking approach to mocker.patch.object for _call.
metaflow/plugins/pypi/pip.py	Converts regex to a raw string for correct escaping.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[greptile-apps]

Greptile Summary

This PR standardizes 64 test files from unittest-style classes to pytest conventions — replacing unittest.mock with pytest-mock, converting class-based tests to top-level functions, extracting shared helpers into fixtures, and adding parametrize ids for clearer output. A single production-code change (pip.py) adds an r-prefix to a regex literal, fixing a DeprecationWarning.

• Test infrastructure: try/finally teardown blocks are replaced by fixtures (isolated_store, clean_store, make_serializer_module); factory fixtures replace module-level helper functions across many test files.
• Test clarity: parametrize ids= are added to ~20 test files; test functions are renamed to be more descriptive; docstrings added throughout.
• One cleanup gap: test_retry_fires_via_real_import_hook in test_serializer_lifecycle.py still performs _interceptor cleanup inline after assertions rather than in a teardown path, removing the try/finally guarantee from the original code.

Confidence Score: 4/5

Safe to merge; the only concern is a test-isolation gap that only manifests when one specific test is already failing.

The vast majority of the 64 changed files are straightforward mechanical conversions that preserve test logic. The one notable gap is in test_serializer_lifecycle.py: test_retry_fires_via_real_import_hook cleans up _interceptor._watched and _interceptor._processed inline after assertions, so if the import hook doesn't fire as expected and an assertion fails, the interceptor is left with a stale watched entry. The original try/finally guaranteed cleanup even on failure; the refactored version does not.

test/unit/test_serializer_lifecycle.py — the test_retry_fires_via_real_import_hook function's interceptor cleanup is not covered by isolated_store and is unreachable on assertion failure.

Important Files Changed

Filename	Overview
test/unit/test_serializer_lifecycle.py	Converts try/finally cleanup blocks to pytest fixtures (isolated_store, make_serializer_module); the interceptor cleanup for test_retry_fires_via_real_import_hook remains inline after assertions, leaving _interceptor state potentially unrestored on assertion failure.
metaflow/plugins/pypi/pip.py	Minor correctness fix: adds r-prefix to regex string in PipPackageNotFound to suppress Python DeprecationWarning for unrecognized escape sequences.
test/unit/test_artifact_serializer.py	Adds clean_store fixture to replace inline try/finally teardown; moves _DualFormatSerializer class earlier; test_priority_tie_lexicographic_fallback is preserved in the refactored module at line 285.
test/unit/test_s3_empty_input.py	Converts unittest-style class hierarchy to standalone pytest functions with a shared s3_instance fixture; adopts mocker.patch.object; cleaner use of tmp_path instead of NamedTemporaryFile with manual unlink.
test/unit/test_add_to_package.py	Replaces module-level helper functions with factory fixtures (make_step, make_flow, make_environment, make_deco, build_pkg, setup_flow_dir); uses tmp_path throughout; logic is preserved.
test/unit/test_graph_structure.py	Cosmetic refactoring: import reordering, heading capitalisation, docstring edits. Introduces a duplicate-word typo in _SingleStepWithConfig's docstring.
test/unit/localbatch/test_localbatch.py	Converts TestBatchAPI, TestDockerExecution, and TestMetaflowE2E unittest classes to top-level pytest functions; adds autouse _require_docker_if_marked fixture to replace per-class skipIf logic.

_{Reviews (5): Last reviewed commit: "added spacing" | Re-trigger Greptile}

[greptile-apps]

Want your agent to iterate on Greptile's feedback? Try greploops.

[talsperre]

Can you fix the pre-commit hook issues?

[talsperre]

Also, this diff is too large - ideally we should be splitting it into multiple PRs that we can tackle this one at a time.

[agsaru]

@talsperre fixed the pre commit. So now do I have to make multiple PRs or what?

[talsperre]

@agsaru I was just saying that let's make each PR more readable - a few modules at a time. So yes, lets split it.

[agsaru]

ok I will do that