Re-add tests/ directory for coverage

[A5rocks]

Making this PR to see if this quick fix works.

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

[A5rocks]

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...

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00000%. Comparing base (77dd921) to head (44a9fff).

Additional details and impacted files

@@               Coverage Diff               @@
##                 main        #3314   +/-   ##
===============================================
  Coverage   100.00000%   100.00000%           
===============================================
  Files             125          128    +3     
  Lines           19251        19269   +18     
  Branches         1304         1303    -1     
===============================================
+ Hits            19251        19269   +18     

Files with missing lines	Coverage Δ
src/trio/_core/_tests/test_thread_cache.py	100.00000% <ø> (ø)
src/trio/_core/_thread_cache.py	100.00000% <ø> (ø)

... and 3 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]

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]

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]

I think this would be nice to merge. Honestly, all this coverage stuff is frustrating and not worth the effort for Trio as it is now... we already have a good enough story here IMO!

[webknjaz]

I still prefer source = ., FWIW

[A5rocks]

I guess I'm confused about how that would work. Like, completely disregarding the exact settings we need, I imagine we want something like:

1. check everything in src and tests is executed (even if nothing uploads a coverage report talking about them)
2. merge all trio paths together, so that the venv -- located at different places on different operating systems -- doesn't affect things

I think this PR accomplishes that. Respectively:

1. src doesn't actually matter since we install everything, so only worry about the importable trio, tests/ is there.
2. tool.coverage.paths.source does this, the extra "src" is for local editable installs (I think? I forgot... that one's iffy)

The part with _trio_check_attrs_aliases is an unfortunate side effect of the fact that we also have our own pytest plugin and need it to be a) importable but b) not within trio.

[jakkdl]

I think this would be nice to merge. Honestly, all this coverage stuff is frustrating and not worth the effort for Trio as it is now... we already have a good enough story here IMO!

sounds good enough to me, agree that coverage has been incredibly frustrating

[webknjaz]

@A5rocks to me, saying that the entire repo is the sources we'd like to measure seems more semantically correct. Hence using "." for that. And then, listing the importable/installable packages would go into the setting that does not allow paths and expects said importables. So source_pkgs should be "trio" (as this is what's being installed). The path mapping is already configured separately so that's irrelevant in this context.
I don't really undersand why _trio_check_attrs_aliases needs to be special-cased at all (as long as it's covered under sources).

What I don't like is that things under source mean maybe a directory, but perhaps an importable — we'll let you guess. This is quite an implicit setting, while source_pkgs allows being explicit about things that are expected to be in the import namespace.

Do you have what didn't work exactly documented anywhere? Maybe some logs? I see the PR description implies that something doesn't work but doesn't really explain what that means.

[webknjaz]

@A5rocks FYI merging this PR broke coverage on 3.14t on windows: https://github.com/python-trio/trio/actions/runs/19958666912/job/57271848880#step:4:1376. It wasn't rebased and so that job never ran here. Using merge queues could've prevented this.

[A5rocks]

I don't really undersand why _trio_check_attrs_aliases needs to be special-cased at all (as long as it's covered under sources).

It's installed as a separate module. (so it's not covered under sources)

This is quite an implicit setting, while source_pkgs allows being explicit about things that are expected to be in the import namespace.

Good point, I should make a PR that does this change.

Do you have what didn't work exactly documented anywhere? Maybe some logs? I see the PR description implies that something doesn't work but doesn't really explain what that means.

Probably all the logs are expired and anyways it's scattered in many places... I forgot why source = ["."] failed, your reasoning for why it should be preferred makes sense to me.

@A5rocks FYI merging this PR broke coverage on 3.14t on windows: https://github.com/python-trio/trio/actions/runs/19958666912/job/57271848880#step:4:1376. It wasn't rebased and so that job never ran here. Using merge queues could've prevented this.

Merge queues unfortunately require only one merge method and we alternate between squash merges and regular merges :(

[A5rocks]

Now that I'm thinking about it, I'm fairly certain the issue with source = ["."] was that it would process trio paths as src/trio but when having it as an importable it would be trio, which would lead to files being marked as uncovered. Probably worth trying again, but... yeah.

[webknjaz]

I don't really undersand why _trio_check_attrs_aliases needs to be special-cased at all (as long as it's covered under sources).

It's installed as a separate module. (so it's not covered under sources)

Which is another sign that it should be under source_pkgs and not sources.

This is quite an implicit setting, while source_pkgs allows being explicit about things that are expected to be in the import namespace.

Good point, I should make a PR that does this change.

This was actually my main point throughout discussions... But in my experience the best config is a combination of source_pkgs with all the importables and source = .. I've been trying to communicate this in the past, which is why it was puzzling that you've gone this route in the PR.

Do you have what didn't work exactly documented anywhere? Maybe some logs? I see the PR description implies that something doesn't work but doesn't really explain what that means.

Probably all the logs are expired and anyways it's scattered in many places... I forgot why source = ["."] failed, your reasoning for why it should be preferred makes sense to me.

Can it be because it wasn't combined with source_pkgs? I've found that it works best with Codecov this way.

@A5rocks FYI merging this PR broke coverage on 3.14t on windows: https://github.com/python-trio/trio/actions/runs/19958666912/job/57271848880#step:4:1376. It wasn't rebased and so that job never ran here. Using merge queues could've prevented this.

Merge queues unfortunately require only one merge method and we alternate between squash merges and regular merges :(

Then, you'll need to require the PR branches to be up-to-date, meaning a rebase each time before merging a PR.

Now that I'm thinking about it, I'm fairly certain the issue with source = ["."] was that it would process trio paths as src/trio but when having it as an importable it would be trio, which would lead to files being marked as uncovered. Probably worth trying again, but... yeah.

That's right. Correctly configured paths should map this properly.

[webknjaz]

@A5rocks this is roughly what I wanted from this PR originally: #3367. Looking at the Codecov report in there, it's what I'd expect.