Skip to content

Conversation

@MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Jun 10, 2025

This is slightly more annoying than I thought. The main reason the tests were in ty_project is that we have access to the ProjectDatabase. We don't have any such Db struct available in ty_python_semantic other than TestDb which is gated behind cfg(test) and integration tests can't enable individual features.

We have a few options here:

  • Make the corpus test a normal unit test: This feels off, especially considering how long they take to run
  • Require a feature for the corpus tests to run: This has the downside that they'll only run when using --all-features.
  • Add a new CorpusDb similar to what we do in ty_test
  • Add a SemanticDb struct in ty_python_semantic and expose it always

I opted for another CorpusDb. This is a bit annoying when adding new methods to the Db trait but we do this only very rarely.

@MichaReiser MichaReiser added testing Related to testing Ruff itself ty Multi-file analysis & type inference labels Jun 10, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jun 10, 2025

mypy_primer results

No ecosystem changes detected ✅

@MichaReiser MichaReiser force-pushed the micha/corpus-test-ty-semantic branch from 667e6a3 to 0c4652b Compare June 10, 2025 11:56
…er__rules__flake8_return__tests__RET504_RET504.py.snap.new
@MichaReiser MichaReiser marked this pull request as ready for review June 10, 2025 11:57
@github-actions
Copy link
Contributor

github-actions bot commented Jun 10, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

@AlexWaygood
Copy link
Member

AlexWaygood commented Jun 10, 2025

Could you also move the corpus itself from ty_project/resources/ to ty_python_semantic/resources/? I think it makes sense for the corpus to be in the same crate as the corpus test

Copy link
Contributor

@carljm carljm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thank you! I agree with @AlexWaygood that preferably the actual corpus would move as well.

@MichaReiser MichaReiser merged commit 5dcfc9f into main Jun 11, 2025
35 checks passed
@MichaReiser MichaReiser deleted the micha/corpus-test-ty-semantic branch June 11, 2025 06:55
dcreager added a commit that referenced this pull request Jun 12, 2025
* main:
  [ty] Add some "inside string" tests for `object.<CURSOR>` completions
  [ty] Pull types on synthesized Python files created by mdtest (#18539)
  Update Rust crate anstyle to v1.0.11 (#18583)
  [`pyupgrade`] Fix `super(__class__, self)` detection in UP008 (super-call-with-parameters) (#18478)
  [ty] Generate the top and bottom materialization of a type (#18594)
  `SourceOrderVisitor` should visit the `Identifier` part of the `PatternKeyword` node (#18635)
  Update salsa (#18636)
  [ty] Update mypy_primer doc (#18638)
  [ty] Improve support for `object.<CURSOR>` completions
  [ty] Add `CoveringNode::find_last`
  [ty] Refactor covering node representation
  [ty] Infer the Python version from `--python=<system installation>` on Unix (#18550)
  [`flake8-return`] Fix `RET504` autofix generating a syntax error (#18428)
  Fix incorrect salsa `return_ref` attribute (#18605)
  Move corpus tests to `ty_python_semantic` (#18609)
  [`pyupgrade`] Don't offer fix for `Optional[None]` in non-pep604-annotation-optional (`UP045)` or non-pep604-annotation-union (`UP007`) (#18545)
  [`pep8-naming`] Suppress fix for `N804` and `N805` if the recommend name is already used (#18472)
  [`ruff`] skip fix for `RUF059` if dummy name is already bound (unused-unpacked-variable) (#18509)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testing Related to testing Ruff itself ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants