move /tests/ to /_tests/ #2628

jakkdl · 2023-04-05T13:55:10Z

Simply renames the directories trio/tests and trio/_core/tests, and updates references to them.

Addresses some (or all?) problems in #274
Fixes the second task in #2625

@njsmith - especially wrt to considerations for #274

jakkdl · 2023-04-05T14:45:44Z

ah, trio/_tests/conftest.py is not parsed for CLI flags (i.e. --runslow) before running. Could e.g. solve that by moving conftest.py into trio/, but that'd make it public.
There's probably other ways of achieving it ... but it's surprisingly tricky, and I've yet to manage something else decent

A5rocks · 2023-04-05T22:25:58Z

I poked at this a bit and adding -p "trio._tests.conftest" seems to get further -- until it complains about adding the plugin twice (second time as normal part of test discovery).

Like the comment in the file says, we can essentially put trio._tests.conftest (which contains all of trio._core._tests.conftest) into a plugin and that'll work fine.

jakkdl · 2023-04-07T10:24:45Z

I poked at this a bit and adding -p "trio._tests.conftest" seems to get further -- until it complains about adding the plugin twice (second time as normal part of test discovery).

Like the comment in the file says, we can essentially put trio._tests.conftest (which contains all of trio._core._tests.conftest) into a plugin and that'll work fine.

huh, I don't get that far:

  File "/home/h/Git/trio/trio/_tests/conftest.py", line 10, in <module>
    from ..testing import trio_test, MockClock
  File "/home/h/Git/trio/trio/testing/__init__.py", line 7, in <module>
    from ._sequencer import Sequencer
  File "/home/h/Git/trio/trio/testing/_sequencer.py", line 11, in <module>
    from .. import Event
ImportError: Error importing plugin "trio._tests.conftest": cannot import name 'Event' from 'trio' (unknown location)

though maybe I've messed up my environment somehow. Feel free to push a commit that makes it a plugin if you can get it to work.

A5rocks · 2023-04-12T00:25:05Z

I should probably mention this now before I forget: we should have a dummy tests.py or something that has a from ._tests import * (or the like, as long as it doesn't re-export for pyright) and a deprecation. Breaking changes are bad, even if we expect nobody uses this.

codecov · 2023-04-13T12:47:36Z

Codecov Report

Merging #2628 (1b42276) into master (855f5fd) will decrease coverage by 0.73%.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2628      +/-   ##
==========================================
- Coverage   92.46%   91.73%   -0.73%     
==========================================
  Files         118      118              
  Lines       16431    16444      +13     
  Branches     2970     2970              
==========================================
- Hits        15193    15085     -108     
- Misses       1124     1244     +120     
- Partials      114      115       +1

Impacted Files	Coverage Δ
trio/_core/_tests/test_asyncgen.py	`99.06% <ø> (ø)`
trio/_core/_tests/test_guest_mode.py	`99.65% <ø> (ø)`
trio/_core/_tests/test_instrumentation.py	`100.00% <ø> (ø)`
trio/_core/_tests/test_io.py	`98.24% <ø> (ø)`
trio/_core/_tests/test_ki.py	`97.83% <ø> (ø)`
trio/_core/_tests/test_local.py	`100.00% <ø> (ø)`
trio/_core/_tests/test_mock_clock.py	`100.00% <ø> (ø)`
trio/_core/_tests/test_multierror.py	`100.00% <ø> (ø)`
...io/_core/_tests/test_multierror_scripts/_common.py	`100.00% <ø> (ø)`
...tests/test_multierror_scripts/apport_excepthook.py	`100.00% <ø> (ø)`
... and 44 more

... and 7 files with indirect coverage changes

jakkdl · 2023-04-13T12:52:05Z

I'm still having some env issues with running the tests myself sometimes, but they seem to pass CI - so it should be fine on a clean install.

Added trio/tests.py and guessed how it was supposed to look :)

A5rocks

Presumably doing import trio.tests will work as a test but I don't think we need one.

trio/tests.py

jakkdl · 2023-04-17T12:39:27Z

Cleaned up and added an import+deprecation test.
I'm a bit confused about codecov wrt trio.testing, but I don't think it's a problem?

Formatting seems to have hit a snag with ... something unrelated to this PR.
and 3.12 continues to fail like usual.

jakkdl · 2023-05-05T09:13:48Z

@A5rocks this one is also ready for review

A5rocks

Cursory review comments, I might find something else in a second review!

trio/_tests/pytest_plugin.py

A5rocks · 2023-05-15T13:02:45Z

trio/tests.py

+    def __getattr__(self, attr: str) -> Any:
+        warn_deprecated(
+            f"trio.tests.{attr}",
+            "0.23.0",


I'm decently sure this deprecation should be kicked off until 0.24.0 -- this will probably get out in a 0.22.0 that removes support for Python 3.7 which will be a major (-> minor cause 0.) bump.

Sounds good, I just copy-pasted another deprecation warning without considering the versioning :)

A5rocks · 2023-05-15T13:05:23Z

FYI master fixes the CI runs, so just merge that in!

jakkdl · 2023-05-16T10:19:13Z

rebased on top of master, and fixed review comments

jakkdl · 2023-05-16T12:01:12Z

I'm a bit confused about codecov wrt trio.testing, but I don't think it's a problem?

Ah. Codecov doesn't see them anymore because trio._tests.pytest_plugin is run with pytest -p, and that must be before codecov gets activated. That's also the first time testing is imported, so on subsequent imports python doesn't reparse what was already covered when pytest_plugin imported it - and codecov therefore doesn't see it.

So we should maybe switch to starting coverage before pytest? https://stackoverflow.com/a/62224494
i.e. switch to coverage run -m pytest ....
I think that would solve the issue

A5rocks · 2023-05-16T12:04:57Z

Sounds good, we want to redo codecov a bit too since it doesn't work with like Windows and whatever ATM. So feel free to change things slightly if necessary!

jakkdl · 2023-05-17T14:50:08Z

uh, looks like codecov isn't turning up as a check at all now. I'll just revert it and we can put a pin into running with codecov -m for the upcoming codecov changes in a separate PR. I'd like this merged so it stops blocking #2629

A5rocks

Looks mostly good, I'd like @njsmith to weigh in cause I'm not too used to changes like this!

A5rocks · 2023-05-18T01:52:16Z

trio/tests.py

+# don't ask which of the _tests imports is necessary
+import trio._tests
+
+from . import _tests


I'm a bit confused by this, won't the namespaces shadow and so the first is useless?

Don't you see the comment?? 😂
I don't remember what caused the confusion back when I was messing around with it. Neither is technically useless atm, as they import the same package as different names. Removing the latter makes no difference to functionality, and removing the first one requires exchanging references to trio._tests to be _tests. But importing from the local package seems superior, so I'll do the latter.

trio/tests.py

jakkdl · 2023-05-18T12:36:18Z

Fixed review comments, removing unnecessary imports. And reading the PR after having forgotten what the code does unsurprisingly lead to some improved comments and tests 😇

njsmith · 2023-05-22T01:40:38Z

trio/tests.py

+
+
+# https://stackoverflow.com/questions/2447353/getattr-on-a-module
+sys.modules[__name__] = TestsDeprecationWrapper()  # type: ignore[assignment]


I'm not sure we really need to be this careful about backcompat here -- surely people aren't actually using trio.tests, right? -- but I guess it's harmless.

Though... I guess we actually want to get rid of trio.tests entirely, so maybe we should issue the warning on import? If someone is importing it but not using it (for some reason), then they won't get a warning, but they'll still get broken in the future when we delete this file.

ah, took me a minute but figured out how to get it to warn on import as well. Only thing it doesn't warn on now that would break is import trio; assert trio.tests - i.e. access of the module, but without importing it, and not accessing any of it's attributes. (I tried changing from using __getattr__ to __getattribute__ but that made no difference in this case). There's probably some way of addressing that, but now we're getting into very weird cases.

jakkdl · 2023-05-22T10:03:46Z

removed the test import from __init__.py, so it's now also invisible to static analysis. Added a corresponding exception for it being a missing symbols in the static analysis tests.

jakkdl · 2023-05-24T12:58:51Z

@A5rocks any reservations about merging this?

jakkdl force-pushed the import_as branch from 7aa7b19 to 3f3b579 Compare April 5, 2023 14:51

jakkdl mentioned this pull request Apr 7, 2023

change imports to from Y import X as X for compatibility with pyright --verifytypes #2629

Merged

3 tasks

jakkdl force-pushed the import_as branch from 3f3b579 to ed5e349 Compare April 13, 2023 12:42

jakkdl force-pushed the import_as branch from ed5e349 to e41e7f4 Compare April 13, 2023 12:49

A5rocks reviewed Apr 13, 2023

View reviewed changes

trio/tests.py Outdated Show resolved Hide resolved

jakkdl force-pushed the import_as branch 2 times, most recently from 1725f04 to 92ae278 Compare April 17, 2023 12:24

A5rocks reviewed May 15, 2023

View reviewed changes

jakkdl force-pushed the import_as branch 2 times, most recently from c12ce07 to 6be34f3 Compare May 16, 2023 10:15

move **/tests/ to **/_tests/

dc37b79

jakkdl force-pushed the import_as branch 2 times, most recently from ce7fddc to 00fe706 Compare May 17, 2023 14:38

add trio.tests module that raises deprecationwarning, add tests

26d9f6e

jakkdl force-pushed the import_as branch from 00fe706 to 26d9f6e Compare May 17, 2023 14:51

jakkdl mentioned this pull request May 17, 2023

use coverage instead of pytest-cov #2645

Closed

A5rocks reviewed May 18, 2023

View reviewed changes

remove unnecessary imports, refactor tests, improve comments

0b5d131

jakkdl force-pushed the import_as branch from be0995e to 0b5d131 Compare May 18, 2023 12:10

njsmith approved these changes May 22, 2023

View reviewed changes

jakkdl force-pushed the import_as branch 2 times, most recently from a74edda to 915c22c Compare May 22, 2023 10:01

jakkdl force-pushed the import_as branch from 915c22c to 927efee Compare May 24, 2023 11:48

now also warns on from trio import tests

1b42276

jakkdl force-pushed the import_as branch from 927efee to 1b42276 Compare May 24, 2023 12:18

A5rocks approved these changes May 24, 2023

View reviewed changes

A5rocks merged commit ef06fba into python-trio:master May 24, 2023

A5rocks mentioned this pull request May 24, 2023

Fix our test layout #274

Open

jakkdl deleted the import_as branch May 24, 2023 15:19



		# https://stackoverflow.com/questions/2447353/getattr-on-a-module
		sys.modules[__name__] = TestsDeprecationWrapper() # type: ignore[assignment]

Uh oh!

move **/tests/ to **/_tests/ #2628

move **/tests/ to **/_tests/ #2628

Uh oh!

Conversation

jakkdl commented Apr 5, 2023

Uh oh!

jakkdl commented Apr 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

A5rocks commented Apr 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jakkdl commented Apr 7, 2023

Uh oh!

A5rocks commented Apr 12, 2023

Uh oh!

codecov bot commented Apr 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jakkdl commented Apr 13, 2023

Uh oh!

A5rocks left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jakkdl commented Apr 17, 2023

Uh oh!

jakkdl commented May 5, 2023

Uh oh!

A5rocks left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

A5rocks May 15, 2023

Choose a reason for hiding this comment

Uh oh!

jakkdl May 16, 2023

Choose a reason for hiding this comment

Uh oh!

A5rocks commented May 15, 2023

Uh oh!

jakkdl commented May 16, 2023

Uh oh!

jakkdl commented May 16, 2023

Uh oh!

A5rocks commented May 16, 2023

Uh oh!

jakkdl commented May 17, 2023

Uh oh!

A5rocks left a comment

Choose a reason for hiding this comment

Uh oh!

A5rocks May 18, 2023

Choose a reason for hiding this comment

Uh oh!

jakkdl May 18, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jakkdl commented May 18, 2023

Uh oh!

njsmith May 22, 2023

Choose a reason for hiding this comment

Uh oh!

jakkdl May 22, 2023

Choose a reason for hiding this comment

Uh oh!

jakkdl commented May 22, 2023

Uh oh!

jakkdl commented May 24, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

move /tests/ to /_tests/ #2628

move /tests/ to /_tests/ #2628

jakkdl commented Apr 5, 2023 •

edited

Loading

A5rocks commented Apr 5, 2023 •

edited

Loading

codecov bot commented Apr 13, 2023 •

edited

Loading