Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 9, 2025

Plan to fix cibuildwheel test command

  • Explore the repository structure and understand the issue
  • Update pyproject.toml to change the test-command in [tool.cibuildwheel] section
  • Address feedback to keep python -m prefix
  • Review the change to ensure it matches requirements
  • Reply to comment
  • Request code review (no issues found)
  • Run security check (passed - configuration change only)
  • Complete the task

Changes Made

Updated the cibuildwheel test command from:

test-command = ["python -m pytest {project}"]

to:

test-command = ["python -m pytest {project}/tests --cov=apyds"]

This change:

  • Keeps the python -m prefix to ensure pytest runs in the correct Python environment
  • Restricts pytest collection to only {project}/tests directory (prevents collecting from bnf/tests/)
  • Adds --cov=apyds flag for coverage parity with local runs

Issue Summary

The original cibuildwheel test command caused pytest to collect tests from all directories including bnf/tests/, which failed with ModuleNotFoundError for apyds_bnf. The fix restricts collection to only the main tests/ directory while maintaining the python -m prefix and adding coverage option.

Security Summary

No security vulnerabilities detected. This is a configuration-only change to the pyproject.toml file.

Original prompt

Change the cibuildwheel test command in pyproject.toml so pytest only collects tests from the tests directory and adds the coverage option.

Background:

  • Current failing job shows cibuildwheel running: python -m pytest /project which causes pytest to collect tests outside the intended tests directory (e.g., bnf/tests) and produced ModuleNotFoundError for apyds_bnf.
  • The project currently has [tool.pytest.ini_options] testpaths = "tests" which works locally, but passing an explicit path to pytest (like {project}) overrides the ini option and causes broad collection.

Requested change:

  • Edit pyproject.toml and update the [tool.cibuildwheel] section to set:

    test-command = ["pytest {project}/tests --cov=apyds"]

Why:

  • Specifying {project}/tests forces pytest to only collect tests under that directory, preventing accidental collection of other test directories in the repository root.
  • Adding --cov=apyds keeps coverage behavior equivalent to local runs.

Caveats / recommendations:

  • This change restricts collection scope but does not install the built wheel. If your tests rely on importing the installed package (recommended), you should consider installing the wheel before running pytest, for example:
    test-command = ["bash", "-lc", "pip install dist/*.whl && pytest --cov=apyds"]
    or
    test-command = ["bash", "-lc", "pip install . && pytest {project}/tests --cov=apyds"]
  • If tests are written to import from the local source tree, the proposed change should fix the immediate issue (no longer collecting tests outside tests/).

What I'll change in the PR:

  • Update pyproject.toml at ref 9633968 in repository USTC-KnowledgeComputingLab/ds to set:
    test-command = ["pytest {project}/tests --cov=apyds"]
  • Include a short commit message: "ci: limit cibuildwheel pytest collection to tests directory"

Please confirm you want me to open the PR with exactly this single-line change. If you want me to also change the test-command to pip install the built wheel before running pytest (recommended), tell me which option you prefer.

This pull request was created as a result of the following prompt from Copilot chat.

Change the cibuildwheel test command in pyproject.toml so pytest only collects tests from the tests directory and adds the coverage option.

Background:

  • Current failing job shows cibuildwheel running: python -m pytest /project which causes pytest to collect tests outside the intended tests directory (e.g., bnf/tests) and produced ModuleNotFoundError for apyds_bnf.
  • The project currently has [tool.pytest.ini_options] testpaths = "tests" which works locally, but passing an explicit path to pytest (like {project}) overrides the ini option and causes broad collection.

Requested change:

  • Edit pyproject.toml and update the [tool.cibuildwheel] section to set:

    test-command = ["pytest {project}/tests --cov=apyds"]

Why:

  • Specifying {project}/tests forces pytest to only collect tests under that directory, preventing accidental collection of other test directories in the repository root.
  • Adding --cov=apyds keeps coverage behavior equivalent to local runs.

Caveats / recommendations:

  • This change restricts collection scope but does not install the built wheel. If your tests rely on importing the installed package (recommended), you should consider installing the wheel before running pytest, for example:
    test-command = ["bash", "-lc", "pip install dist/*.whl && pytest --cov=apyds"]
    or
    test-command = ["bash", "-lc", "pip install . && pytest {project}/tests --cov=apyds"]
  • If tests are written to import from the local source tree, the proposed change should fix the immediate issue (no longer collecting tests outside tests/).

What I'll change in the PR:

  • Update pyproject.toml at ref 9633968 in repository USTC-KnowledgeComputingLab/ds to set:
    test-command = ["pytest {project}/tests --cov=apyds"]
  • Include a short commit message: "ci: limit cibuildwheel pytest collection to tests directory"

Please confirm you want me to open the PR with exactly this single-line change. If you want me to also change the test-command to pip install the built wheel before running pytest (recommended), tell me which option you prefer.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

@hzhangxyz hzhangxyz marked this pull request as ready for review December 9, 2025 08:31
Copilot AI review requested due to automatic review settings December 9, 2025 08:31
Copy link
Member

@hzhangxyz hzhangxyz left a comment

Choose a reason for hiding this comment

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

python -m 别丢了

Copilot AI changed the title [WIP] Update cibuildwheel test command for pytest configuration ci: limit cibuildwheel pytest collection to tests directory Dec 9, 2025
Copilot AI requested a review from hzhangxyz December 9, 2025 08:32
Copy link
Contributor Author

Copilot AI commented Dec 9, 2025

python -m 别丢了

Done. Updated to keep python -m prefix in da6fc08.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the cibuildwheel test command to explicitly restrict pytest test collection to the tests/ directory, preventing unintended collection of tests from other subdirectories like bnf/tests/. The change addresses a ModuleNotFoundError that occurred when cibuildwheel ran python -m pytest {project}, which caused pytest to collect tests from all directories, including bnf/tests/ which depends on a different package (apyds_bnf).

Key Changes:

  • Modified the cibuildwheel test-command to explicitly target {project}/tests instead of {project}
  • Added explicit --cov=apyds flag to maintain coverage reporting during wheel testing

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

windows.archs = ["AMD64"]
test-requires = ["pytest", "pytest-cov"]
test-command = ["python -m pytest {project}"]
test-command = ["python -m pytest {project}/tests --cov=apyds"]

This comment was marked as outdated.

@hzhangxyz hzhangxyz merged commit a8e31bd into main Dec 9, 2025
79 checks passed
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