Skip to content

Re-add tests/ directory #3314

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

A5rocks
Copy link
Contributor

@A5rocks A5rocks commented Aug 7, 2025

Making this PR to see if this quick fix works.

I want to double check that source = ["."] doesn't work before merging this.

@A5rocks A5rocks mentioned this pull request Aug 7, 2025
@A5rocks
Copy link
Contributor Author

A5rocks commented Aug 7, 2025

Hm that didn't work. 🤔

EDIT: nevermind it did work, I don't know why the coverage report step of the Cython tests didn't show me what I was looking for. I guess maybe I somehow skipped the line while reading...

Copy link

codecov bot commented Aug 7, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00000%. Comparing base (f8a51b6) to head (b871153).

Additional details and impacted files
@@               Coverage Diff               @@
##                 main        #3314   +/-   ##
===============================================
  Coverage   100.00000%   100.00000%           
===============================================
  Files             125          127    +2     
  Lines           19253        19261    +8     
  Branches         1304         1303    -1     
===============================================
+ Hits            19253        19261    +8     
Files with missing lines Coverage Δ
src/trio/_core/_tests/test_thread_cache.py 100.00000% <ø> (ø)
src/trio/_core/_thread_cache.py 100.00000% <ø> (ø)

... and 2 files with indirect coverage changes

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

@webknjaz
Copy link
Member

webknjaz commented Aug 7, 2025

The problem with source is that it's ambiguously selecting either importables or directories. And you don't really know what you'll get upfront. But with the CWD, it's always a dir. And source_pkgs specifically recognizes importables.

This usually works best with Codecov, and pytest --cov (without a value set).

But since something wasn't right in trio, I've been postponing the original merge.

I'd recommend comparing the coverage. xml files produced before the PR and after. I suspect the base package dir attribute is different with your change.

@A5rocks
Copy link
Contributor Author

A5rocks commented Aug 7, 2025

Note for a later search: for some reason test_clear_thread_cache_after_fork is timing out.

The problem with source is that it's ambiguously selecting either importables or directories.

Yeah I tried to add a / just to ensure this is fine... We'll see. Hopefully codecov failure was just because of CI flakiness.

@A5rocks A5rocks added the skip newsfragment Newsfragment is not required label Aug 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip newsfragment Newsfragment is not required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants