Skip to content

Conversation

@reinecke
Copy link
Collaborator

@reinecke reinecke commented Nov 9, 2025

This change re-shuffles the CI so that:

  • A "smoke test" set of builds are run on one OS and generate coverage
  • A separate "build and test only" set of platforms/python version run off the sdist (currently mingw)
  • All wheels are built after smoke tests pass
  • The test suite is run for each OS/python combo against the wheels

Ideally this would take the suggestion from @JeanChristopheMorinPerso and run against sdist - that change hasn't been made.
Update: after investigation, cibuildwheel isn't really meant to happen from sdist sources. I did a pass to make our build and test only phase run off it though - since those platforms would be running off sdist in the wild.

Additionally, if we could capture coverage from one of the cibuildwheel runs, we could probably eliminate the smoke test phase.

  • Consider dropping the smoke test and just harvest coverage from testing the wheels
    • Update: we decided against this because you need particular build options to generate needed coverage artifacts
  • Investigate having the wheel build to happen off the sdist
  • Currently the build and test only phase is hitting a test error - getting it to find the sdist tar.gz when the name is indeterminate (includes the version) is proving tricky
  • Re-shuffle the order of repo checkout and wheel install in the wheel test action to match the test only step
  • Reduce the matrix on the smoketest build so that it only does the one it needs for coverage - consider renaming explicitly as "generate_coverage" or similar

Fixes #1027
This supersedes #1961 - the changes have been added to this PR

@github-actions github-actions bot added the ci label Nov 9, 2025
@reinecke reinecke marked this pull request as ready for review November 9, 2025 21:52
@reinecke reinecke marked this pull request as draft November 9, 2025 21:52
@reinecke reinecke marked this pull request as ready for review November 9, 2025 21:52
@codecov-commenter
Copy link

codecov-commenter commented Nov 9, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.15%. Comparing base (ad449d8) to head (9ef3995).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1977      +/-   ##
==========================================
+ Coverage   85.13%   85.15%   +0.01%     
==========================================
  Files         181      181              
  Lines       12783    12783              
  Branches     1206     1206              
==========================================
+ Hits        10883    10885       +2     
+ Misses       1717     1715       -2     
  Partials      183      183              
Flag Coverage Δ
py-unittests 85.15% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ad449d8...9ef3995. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@reinecke reinecke marked this pull request as draft November 10, 2025 00:21
…ist resolution, made wheels no longer depend on smoketest

Signed-off-by: Eric Reinecke <[email protected]>
Signed-off-by: Eric Reinecke <[email protected]>
…act is available for install and the repo is subsequently used for testing

Signed-off-by: Eric Reinecke <[email protected]>
@reinecke reinecke marked this pull request as ready for review November 12, 2025 22:38
@reinecke
Copy link
Collaborator Author

I'm ready for feedback on this - just the one open questions about whether or not we really need the smoketests or if I should move the coverage reports to the tests we run on the equivalent wheel.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Test pass for build artifacts

2 participants