Skip to content

Fix cross-platform test fixture failures on Windows - #1707

Open
hardikkaurani wants to merge 2 commits into
weecology:mainfrom
hardikkaurani:fix-cross-platform-test-fixtures
Open

hardikkaurani wants to merge 2 commits into
weecology:mainfrom
hardikkaurani:fix-cross-platform-test-fixtures

Conversation

@hardikkaurani

@hardikkaurani hardikkaurani commented Sep 6, 2026 •

Copy link
Copy Markdown

Description

This PR resolves cross-platform test fixture failures on Windows by replacing POSIX-specific shell commands with Python standard library operations.

Problem

In test/test_retriever.py and test/test_regression.py, test setup and teardown fixtures (setup_module, teardown_module, and test_setup_functions) invoked POSIX-specific commands via subprocess.call(['cp', '-r', ...]) and subprocess.call(['rm', '-r', ...]).

On Windows systems where POSIX binaries are not natively available in PATH, this resulted in FileNotFoundError: [WinError 2] The system cannot find the file specified, causing all tests in these modules to fail during fixture initialization.

Proposed Changes

  • Replaced subprocess.call(['cp', '-r', ...]) with shutil.copytree() in test/test_retriever.py and test/test_regression.py.
  • Replaced subprocess.call(['rm', '-r', ...]) with shutil.rmtree() and os.remove() for directory and file cleanups.
  • Pre-cleaned destination paths prior to copying to maintain compatibility with Python 3.7+ without requiring the Python 3.8+ dirs_exist_ok keyword argument.

Verification

  • Ran pytest test/test_retriever.py on Windows with Python 3.11.9: fixture setup and teardown executed successfully without FileNotFoundError.
  • Ran pytest test/test_regression.py on Windows: fixture setup, teardown, and SQLite/CSV regression tests completed successfully.
  • Verified that git diff --check reports no whitespace or formatting regressions.

Copilot AI lite review requested due to automatic review settings September 6, 2026 06:09

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.

🟢 Approval recommended

The changes are test-only and correctly replace POSIX-specific commands with cross-platform Python equivalents, with only a minor cleanup nit noted.

Pull request overview

This PR updates test fixtures to be cross-platform (especially Windows) by removing reliance on POSIX cp/rm shell commands and using Python’s standard library for filesystem setup/cleanup.

Changes:

  • Replaced subprocess.call(['cp', '-r', ...]) with shutil.copytree() in test module setup to copy test/raw_data into the expected raw_data location.
  • Replaced subprocess.call(['rm', '-r', ...]) with shutil.rmtree() / os.remove() in teardown and related cleanup paths.
  • Added pre-cleaning of destination paths to keep compatibility with Python versions that don’t support dirs_exist_ok.
File summaries
File Description
test/test_retriever.py Switches fixture setup/teardown and test cleanup from POSIX shell commands to shutil/os operations for Windows compatibility.
test/test_regression.py Applies the same cross-platform filesystem changes to regression test setup/teardown and SQLite cleanup.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread test/test_retriever.py Outdated
Comment on lines 5 to 6
import shutil
import subprocess
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