Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CI] Run tests against pre-built/installed version of setuptools #3049

Draft
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

abravalheri
Copy link
Contributor

@abravalheri abravalheri commented Jan 25, 2022

Motivation

This an attempt to implement the approach described in #3015 (review), i.e. run a CI pipeline that tests the pre-built/installed version of the package.

Summary of changes

  • Make tests independent of the file location / relocatable
    • "in-package" tests are incompatible with the proposed test approach, so we need to mv them before running the test suite (see #2318 (comment)).
  • Rename setuptools/tests/{fixtures.py => conftest.py}
    • This make it possible to run the tests transparently in the CI and in the development machine with usedevelop=True.
  • Stablish a proper CI pipeline, where package is build first and the test suite runs against the installed wheel.
    • To workaround the problems intrinsic to the flat-layout with in-package tests (see #2318 (comment)), during tests the folders {setuptools,pkg_resources}/tests are moved to a top level directory and the {setuptools,pkg_resources} folders are removed.
  • Disable pytest-perf for PyPy (see #3015 (comment))

Closes #2318

Pull Request Checklist

Pytest automatically load `conftest.py` files from directory tests,
so it is not necessary to use an in-tree pytest plugin for that.

This issue was considered a blocker in
pypa#2318.
… if modules are imported before the tested code runs.
Make tests work even if they are unaware what is exactly the folder
organisation in the project (i.e. use relative imports and rely more on
__name__)
@abravalheri
Copy link
Contributor Author

abravalheri commented Jan 25, 2022

@webknjaz I could not find a solution for the test coverage using this approach...

Even if I move the source directories to a folder that is not automatically added to the PYTHONPATH (instead of removing it) and maintain 2 separated config files (one for running tox with develop=True and other just for the CI), the files reported to the coverage aggregation service will not exist in the repository...

Do you have any suggestion about that?


Edit: it seems that codecov supports path fixing, but I am struggling to make this work.


Edit: I solved it using path fixing. The previous .codecov.yml file had a little problem (#3053) that was preventing the configuration to take place.

default:
threshold: 0.5%
fixes:
- "src/::" # reduce root e.g., "src/setuptools/" => "setuptools/"
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is the right solution. This setting seems to change how files on Codecov are mapped to GH.

--installpkg '${{ needs.prepare.outputs.wheel-distribution }}'
-- --cov-report xml --cov-report term --cov-config .coveragerc.ci
- name: Publish coverage
uses: codecov/codecov-action@v1
Copy link
Member

Choose a reason for hiding this comment

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

This is already deprecated, v3 should be used instead.

uses: codecov/codecov-action@v1
with:
flags: >- # Mark which lines are covered by which envs
cygwin
Copy link
Member

Choose a reason for hiding this comment

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

These should be comma-separated

Suggested change
cygwin
cygwin,

@@ -5,6 +5,7 @@ author = Python Packaging Authority
author_email = distutils-sig@python.org
description = Easily download, build, install, upgrade, and uninstall Python packages
long_description = file:README.rst
long_description_content_type = text/x-rst
Copy link
Member

Choose a reason for hiding this comment

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

It'd be great to extract such things into separate PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Project should be tested as installed
2 participants