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

Support alternatives to trio.run() for pytest-trio #1698

Closed
wants to merge 1 commit into from

Conversation

altendky
Copy link
Member

@altendky altendky commented Aug 27, 2020

Draft for:

  • Generally concluding the other PRs are going to work
    • Since moving trio_test() to pytest-trio, the other PRs no longer depend on this one.
  • Maybe deprecate trio_test(), maybe just delete it?

Relates to:

@altendky altendky marked this pull request as draft August 27, 2020 02:43
@codecov
Copy link

codecov bot commented Aug 27, 2020

Codecov Report

Merging #1698 into master will decrease coverage by 0.01%.
The diff coverage is 76.47%.

@@            Coverage Diff             @@
##           master    #1698      +/-   ##
==========================================
- Coverage   99.61%   99.59%   -0.02%     
==========================================
  Files         115      115              
  Lines       14445    14449       +4     
  Branches     1106     1107       +1     
==========================================
+ Hits        14389    14391       +2     
- Misses         41       42       +1     
- Partials       15       16       +1     
Impacted Files Coverage Δ
trio/testing/_trio_test.py 80.00% <76.47%> (-7.50%) ⬇️

@njsmith
Copy link
Member

njsmith commented Aug 27, 2020

What would you say to copying this code into pytest-trio instead, and fixing it there? It's sort of been on my todo list anyway, and doing it now seems like it would simplify your life by consolidating all the changes into one project...

@altendky
Copy link
Member Author

Whatever, I don't particularly care at this point so I'll plan on moving it. Or, did you more literally mean copy and leave this copy alone? It is accessible via the 'public path' of trio.testing.trio_test() and is also in the docs, albeit as a stub. Should this be treated with a full deprecation process?

https://trio.readthedocs.io/en/stable/reference-testing.html#test-harness-integration
image

@altendky
Copy link
Member Author

https://github.com/search?q=%22pyfuncitem.obj+%3D+trio_test%28pyfuncitem.obj%29%22&type=Code
Looks like there's a conftest.py that's been copied around for a few years that does reference trio_test() (including trimeter...) so deprecation it is. Unless it's just going to stick around? (you did say 'copy')

@altendky
Copy link
Member Author

Or maybe I'm just jumping way ahead of myself with this because trio_test() is also used throughout Trio's own tests. Maybe it's not worth even deprecating. In the mean time trimeter's CI might be fixed. :]

@njsmith
Copy link
Member

njsmith commented Sep 1, 2020

Yeah, idk, maybe we should deprecate and remove it, but we don't need to make a decision right now, now that this is off your critical path.

@njsmith njsmith closed this Sep 1, 2020
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