Skip to content

[red-knot] More comprehensive is_assignable_to tests#15353

Merged
sharkdp merged 8 commits intomainfrom
david/migrate-assignable_to-tests
Jan 8, 2025
Merged

[red-knot] More comprehensive is_assignable_to tests#15353
sharkdp merged 8 commits intomainfrom
david/migrate-assignable_to-tests

Conversation

@sharkdp
Copy link
Contributor

@sharkdp sharkdp commented Jan 8, 2025

Summary

This changeset migrates all existing is_assignable_to tests to a Markdown-based test. It also increases our test coverage in a hopefully meaningful way (not claiming to be complete in any sense). But at least I found and fixed one bug while doing so.

Test Plan

Ran property tests to make sure the new test succeeds after fixing it.

This changeset migrates all existing `is_assignable_to` tests to a
Markdown-based test. It also increases our test coverage in a hopefully
meaningful way. At least I found (and fixed) one bug while doing so.
@sharkdp sharkdp added the ty Multi-file analysis & type inference label Jan 8, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jan 8, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Fantastic!

@AlexWaygood AlexWaygood added the testing Related to testing Ruff itself label Jan 8, 2025
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.

Love it, thank you!!

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Some suggested commentary. But I don't mind if you want to leave it to a followup! (And I'm happy to do the followup.) Feel free to merge!

static_assert(is_assignable_to(type[Unknown], Meta))
```

## Tuple types
Copy link
Member

Choose a reason for hiding this comment

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

we don't have any gradual tuples in this seciton, and I wonder if we should (e.g. tuple[Any, Literal[2]] should be assignable to tuple[int, int] but not to tuple[int, str]). Doesn't need to be done in this PR, though

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did that now, and also added tests with gradual types in the union and intersection types sections.

@sharkdp sharkdp force-pushed the david/migrate-assignable_to-tests branch from cd753c6 to 71aab38 Compare January 8, 2025 18:33
@sharkdp sharkdp force-pushed the david/migrate-assignable_to-tests branch from 71aab38 to ffaecb8 Compare January 8, 2025 19:18
@sharkdp sharkdp merged commit beb8e2d into main Jan 8, 2025
21 checks passed
@sharkdp sharkdp deleted the david/migrate-assignable_to-tests branch January 8, 2025 19:25
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.

3 participants