[ty] add tests for workspaces by Gankra · Pull Request #21741 · astral-sh/ruff

Gankra · 2025-12-01T21:07:32Z

Here are a bunch of (variously failing and passing) mdtests that reflect the kinds of issues people encounter when running ty over an entire workspace without sufficient hand-holding (especially because in the IDE it is unclear how to provide that hand-holding).

astral-sh-bot · 2025-12-01T21:09:31Z

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

astral-sh-bot · 2025-12-01T21:11:24Z

`mypy_primer` results

Changes were detected when running on open source projects

pandas-stubs (https://github.com/pandas-dev/pandas-stubs)
+ pandas-stubs/_typing.pyi:1207:16: warning[unused-ignore-comment] Unused blanket `type: ignore` directive
- Found 5814 diagnostics
+ Found 5815 diagnostics

rotki (https://github.com/rotki/rotki)
- rotkehlchen/accounting/structures/processed_event.py:85:75: warning[unused-ignore-comment] Unused blanket `type: ignore` directive
- rotkehlchen/api/rest.py:1039:73: warning[unused-ignore-comment] Unused blanket `type: ignore` directive
- Found 2040 diagnostics
+ Found 2038 diagnostics

No memory usage changes detected ✅

Gankra · 2025-12-01T21:30:04Z

Note that some real-world examples are combinations of these. For instance "Tests Directory With Overlapping Names" actually works perfectly, but it fails in pyx because it's actually technically hitting "Current Directory Is Invalid Module Name" (because we try to resolve relative imports like .setup to aproj.tests.setup, which in pyx's case ends up being a-proj.tests.setup, which is rejected).

Some approaches to workspaces may dodge the invalid-module-name situation, some may not. I tried to get a good coverage of interesting cases that different solutions would show differences on.

MichaReiser · 2025-12-02T08:11:30Z

crates/ty_python_semantic/resources/mdtest/import/workspaces.md

+While you can't syntactically refer to a module with an invalid name (i.e. one with a `-`, or that
+has the same name as a keyword) there are plenty of situations where a module with an invalid name
+can be run. For instance `python my-script.py` and `python my-proj/main.py` both work, even though


Yes, the way I think about it is that you can run a python script with an (mostly?) arbitrary name but whether you can import it depends on the segments relative to the search roots.

MichaReiser · 2025-12-02T08:11:47Z

crates/ty_python_semantic/resources/mdtest/import/workspaces.md

+Also, a sufficiently motivated programmer can technically use `importlib.import_module` which takes
+strings and does in fact allow syntactically invalid module names.


okay, I didn't know that

MichaReiser · 2025-12-02T08:14:39Z

crates/ty_python_semantic/resources/mdtest/import/workspaces.md

+z: int = 2
+```
+
+## Multiple Projects


Long term, I think we want a different solution to correctly support cases where the projects have different configurations (e.g. enabled / disabled some rules). But that shouldn't prevent us from doing better when there's no member-level configuration.

MichaReiser · 2025-12-02T08:16:24Z

crates/ty_python_semantic/resources/mdtest/import/workspaces.md

+[environment]
+# This is similar to what we would compute for installed editables


Those tests work because we also apply our default environment.root paths in mdtests?

I believe so

carljm

Thanks for adding these tests!

carljm · 2025-12-02T17:08:34Z

crates/ty_python_semantic/resources/mdtest/import/workspaces.md

+# error: [unresolved-import]
+from .mod1 import x
+
+# error: [unresolved-import]
+from . import mod2


Both of these imports fail at runtime with ImportError: attempted relative import with no known parent package if you execute python my-main.py.

This is true (both at runtime and in ty today) even if we rename my-main.py to just main.py -- that is, it's unrelated to the use of an invalid module name. It's because you cannot relative-import top-level modules.

The only way these two imports can work is if all of these modules are imported as part of the same top-level parent package (that is, the directory containing these files is a Python package, and its parent directory is on the search path). (With the name my-main.py, the only way that can happen is via importlib.import_module.)

In other words, as this test is currently written, I think the above TODO is wrong, and these imports should error. If we add an outer containing directory and place it on the search path, then I think these imports should work (and already would with a valid module name). With that change, I think the TODO to make them also work with an invalid module name would be correct.

Great callout -- I've put everything but mod3 under a tests/ directory and renamed my-main.py to my-mod.py, and made a clarifying comment that in this case we're assuming importlib shenanigans.

(We in fact have had one user complain that they had a random def.py that they just importlib.import_module but we wouldn't resolve relative imports from it)

carljm · 2025-12-02T17:53:38Z

crates/ty_python_semantic/resources/mdtest/import/workspaces.md

+
+# error: [unresolved-import]
+from . import mod2
+import mod3


The only scenario in which this import can work, and the two imports above also work, is overlapping search paths. The two relative imports can only work if this is all part of a containing package (as described above); this import can only work if these are all top-level modules. Overlapping search paths is the only way those two things can both be true.

(Fix mentioned in previous comment also handles this)

carljm · 2025-12-02T17:59:00Z

crates/ty_python_semantic/resources/mdtest/import/workspaces.md

+
+## Multiple Projects
+
+It's common for a monorepo to define many separate projects that may or may not depend on eachother


Nit: "eachother" is not a word, it's two words. (I assumed this was a typo but then saw multiple other occurrences below. Not worth fixing alone but if we're changing this file anyway we may as well.)

Suggested change

It's common for a monorepo to define many separate projects that may or may not depend on eachother

It's common for a monorepo to define many separate projects that may or may not depend on each other

~~it's Canadian English~~

carljm · 2025-12-02T18:25:23Z

crates/ty_python_semantic/resources/mdtest/import/workspaces.md

+# This is similar to what we would compute for installed editables
+extra-paths = ["aproj/src/", "bproj/src/"]


The use of extra-paths here is significantly different from an installed editable, in that extra-paths are higher priority than environment.roots, while editable installs are lower priority. This seems like it could make a noticeable difference in the behavior of these tests, particularly those that test the interaction between same-named "outer" and "inner" packages.

It would be a bit more boilerplate, but I think these tests could better reflect a realistic scenario if they instead used python = config and then created .pth files in the fake site-packages that pointed to aproj/src, bproj/src -- that would actually get those paths added as editable installs. import/site_packages_discovery.md demonstrates this, minus the .pth file part.

Also a great point, I misremembered the order of extra and first-party.

I think per discussion of "we should be auto-sorting nested search-paths" I probably want to have at least one test that uses editables and one that uses extra-paths (I imagine some users will find extra-paths, do this kind of thing manually, and it will behoove us to untangle it).

carljm · 2025-12-02T18:32:48Z

crates/ty_python_semantic/resources/mdtest/import/workspaces.md

+```py
+# TODO: there should be no errors in this file.
+
+# error: [unresolved-import]


So I guess what happens here is we decide the package we should import is a.tests.setup, but then (because ./a/src/ is a higher priority search path than .) we go looking for a/src/a/tests/setup.py and don't find it.

This behavior seems correct (i.e. matches runtime)? Except that as discussed above, a more realistic test would not use extra-paths, which would make . higher priority than ./a/src/ and thus totally change the behavior of this test.

crates/ty_python_semantic/resources/mdtest/import/workspaces.md

carljm · 2025-12-02T18:40:13Z

crates/ty_python_semantic/resources/mdtest/import/workspaces.md

+The same situation as the previous test but `tests/__init__.py` is also defined, in case that
+complicates the situation.


I don't think it can make a difference to this test. In theory testing various cases makes sense, but there are an infinite number of possible scenarios to test, and I'm not sure it scales to sort of randomly test little tweaks "in case", even if we haven't ever seen evidence (i.e. a failure in real code) that it makes a difference. So I'm kind of not convinced we should have both this test and the previous one, unless there's an observed behavior difference.

This particular case was because I was perplexed by seeing this random variation in pyx's codebase and got a bit paranoid about whether it mattered if tests was a namespace package or a normal package. I think the __init__.py version is "strictly harder" for us to handle, so I should probably only have that one.

carljm · 2025-12-02T18:40:26Z

crates/ty_python_semantic/resources/mdtest/import/workspaces.md

+
+### Tests Package Absolute Importing `main.py`
+
+The same as the previous case but `tests/__init__.py` exists in case that causes different issues.


Same comment as above.

crates/ty_python_semantic/resources/mdtest/import/workspaces.md

my-script.py

* origin/main: (67 commits) Move `Token`, `TokenKind` and `Tokens` to `ruff-python-ast` (#21760) [ty] Don't confuse multiple occurrences of `typing.Self` when binding bound methods (#21754) Use our org-wide Renovate preset (#21759) Delete `my-script.py` (#21751) [ty] Move `all_members`, and related types/routines, out of `ide_support.rs` (#21695) [ty] Fix find-references for import aliases (#21736) [ty] add tests for workspaces (#21741) [ty] Stop testing the (brittle) constraint set display implementation (#21743) [ty] Use generator over list comprehension to avoid cast (#21748) [ty] Add a diagnostic for prohibited `NamedTuple` attribute overrides (#21717) [ty] Fix subtyping with `type[T]` and unions (#21740) Use `npm ci --ignore-scripts` everywhere (#21742) [`flake8-simplify`] Fix truthiness assumption for non-iterable arguments in tuple/list/set calls (`SIM222`, `SIM223`) (#21479) [`flake8-use-pathlib`] Mark fixes unsafe for return type changes (`PTH104`, `PTH105`, `PTH109`, `PTH115`) (#21440) [ty] Fix auto-import code action to handle pre-existing import Enable PEP 740 attestations when publishing to PyPI (#21735) [ty] Fix find references for type defined in stub (#21732) Use OIDC instead of codspeed token (#21719) [ty] Exclude `typing_extensions` from completions unless it's really available [ty] Fix false positives for `class F(Generic[*Ts]): ...` (#21723) ...

add tests for workspaces

9c6dd1f

Gankra requested review from AlexWaygood, carljm, dcreager and sharkdp as code owners December 1, 2025 21:07

Gankra added the internal An internal refactor or improvement label Dec 1, 2025

Gankra requested a review from MichaReiser as a code owner December 1, 2025 21:07

Gankra added the ty Multi-file analysis & type inference label Dec 1, 2025

AlexWaygood removed their request for review December 1, 2025 22:09

fixup

b52cc05

MichaReiser approved these changes Dec 2, 2025

View reviewed changes

sharkdp removed their request for review December 2, 2025 08:19

Gankra merged commit 015ab9e into main Dec 2, 2025
41 checks passed

Gankra deleted the gankra/add-tests branch December 2, 2025 11:43

carljm reviewed Dec 2, 2025

View reviewed changes

This comment was marked as off-topic.

Sign in to view

		Also, a sufficiently motivated programmer can technically use `importlib.import_module` which takes
		strings and does in fact allow syntactically invalid module names.

		[environment]
		# This is similar to what we would compute for installed editables


		## Multiple Projects

		It's common for a monorepo to define many separate projects that may or may not depend on eachother

		# This is similar to what we would compute for installed editables
		extra-paths = ["aproj/src/", "bproj/src/"]

		The same situation as the previous test but `tests/__init__.py` is also defined, in case that
		complicates the situation.


		### Tests Package Absolute Importing `main.py`

		The same as the previous case but `tests/__init__.py` exists in case that causes different issues.

Conversation

Gankra commented Dec 1, 2025

Uh oh!

astral-sh-bot bot commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Diagnostic diff on typing conformance tests

Uh oh!

astral-sh-bot bot commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

mypy_primer results

Uh oh!

Gankra commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

carljm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carljm Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Gankra Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

This comment was marked as off-topic.

This comment was marked as off-topic.

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

astral-sh-bot bot commented Dec 1, 2025 •

edited

Loading

astral-sh-bot bot commented Dec 1, 2025 •

edited

Loading

`mypy_primer` results

Gankra commented Dec 1, 2025 •

edited

Loading

carljm Dec 2, 2025 •

edited

Loading

Gankra Dec 3, 2025 •

edited

Loading