Skip to content

Fix broken red-knot property tests#16574

Merged
AlexWaygood merged 1 commit intomainfrom
alex/fix-property-tests
Mar 9, 2025
Merged

Fix broken red-knot property tests#16574
AlexWaygood merged 1 commit intomainfrom
alex/fix-property-tests

Conversation

@AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented Mar 9, 2025

Summary

Fixes #16566, fixes #16575

The semantics of Type::class_member changed in #16416, but the property-test infrastructure was not updated. That means that the property tests were panicking on the second expect_type call here:

Ty::BuiltinsBoundMethod { class, method } => {
let builtins_class = builtins_symbol(db, class).symbol.expect_type();
let function = builtins_class
.class_member(db, method.into())
.symbol
.expect_type();
create_bound_method(db, function, builtins_class)

With the somewhat unhelpful message:

Expected a (possibly unbound) type, not an unbound symbol

Applying this patch, and then running QUICKCHECK_TESTS=1000000 cargo test --release -p red_knot_python_semantic -- --ignored types::property_tests::stable::equivalent_to_is_reflexive showed clearly that it was no longer able to find any methods on any classes due to the change in semantics of Type::class_member:

--- a/crates/red_knot_python_semantic/src/types/property_tests.rs
+++ b/crates/red_knot_python_semantic/src/types/property_tests.rs
@@ -27,7 +27,7 @@
 use std::sync::{Arc, Mutex, MutexGuard, OnceLock};
 
 use crate::db::tests::{setup_db, TestDb};
-use crate::symbol::{builtins_symbol, known_module_symbol};
+use crate::symbol::{builtins_symbol, known_module_symbol, Symbol};
 use crate::types::{
     BoundMethodType, CallableType, IntersectionBuilder, KnownClass, KnownInstanceType,
     SubclassOfType, TupleType, Type, UnionType,
@@ -150,10 +150,11 @@ impl Ty {
             Ty::BuiltinsFunction(name) => builtins_symbol(db, name).symbol.expect_type(),
             Ty::BuiltinsBoundMethod { class, method } => {
                 let builtins_class = builtins_symbol(db, class).symbol.expect_type();
-                let function = builtins_class
-                    .class_member(db, method.into())
-                    .symbol
-                    .expect_type();
+                let Symbol::Type(function, ..) =
+                    builtins_class.class_member(db, method.into()).symbol
+                else {
+                    panic!("no method `{method}` on class `{class}`");
+                };
 
                 create_bound_method(db, function, builtins_class)
             }

This PR updates the property-test infrastructure to use Type::member rather than Type::class_member.

Test Plan

  • Ran QUICKCHECK_TESTS=1000000 cargo test --release -p red_knot_python_semantic -- --ignored types::property_tests::stable successfully
  • Checked that there were no remaining uses of Type::class_member in property_tests.rs

@AlexWaygood AlexWaygood added bug Something isn't working testing Related to testing Ruff itself ty Multi-file analysis & type inference labels Mar 9, 2025
Copy link
Contributor

@sharkdp sharkdp left a comment

Choose a reason for hiding this comment

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

Thank you @AlexWaygood

@AlexWaygood AlexWaygood merged commit c970b79 into main Mar 9, 2025
21 checks passed
@AlexWaygood AlexWaygood deleted the alex/fix-property-tests branch March 9, 2025 17:40
dcreager added a commit that referenced this pull request Mar 10, 2025
* main:
  [red-knot] Add support for calling `type[…]` (#16597)
  Update migration guide with the new `ruff.configuration` (#16567)
  [red-knot] Add 'mypy_primer' workflow (#16554)
  Update Rust crate indoc to v2.0.6 (#16585)
  Update Rust crate syn to v2.0.100 (#16590)
  Update Rust crate thiserror to v2.0.12 (#16591)
  Update Rust crate serde_json to v1.0.140 (#16589)
  Update Rust crate quote to v1.0.39 (#16587)
  Update Rust crate serde to v1.0.219 (#16588)
  Update Rust crate proc-macro2 to v1.0.94 (#16586)
  Update Rust crate anyhow to v1.0.97 (#16584)
  Update dependency ruff to v0.9.10 (#16593)
  Update Rust crate unicode-ident to v1.0.18 (#16592)
  [red-knot] Do not ignore typeshed stubs for 'venv' module (#16596)
  [red-knot] Reduce Salsa lookups in `Type::find_name_in_mro` (#16582)
  Fix broken red-knot property tests (#16574)
  [red-knot] Consistent spelling of "metaclass" and "meta-type" (#16576)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working 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.

Daily property test run failed on Sun Mar 09 2025 Daily property test run failed on Sat Mar 08 2025

2 participants