Fix min test case by doing mroe validation

carbotaniuman · carbotaniuman · commit 49b5d379055e · 2020-11-11T16:27:02.000-06:00
diff --git a/Lib/test/test_builtin.py b/Lib/test/test_builtin.py
@@ -927,8 +927,6 @@ def __getitem__(self, index):
         self.assertEqual(max(data, key=f),
                          sorted(reversed(data), key=f)[-1])
 
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     def test_min(self):
         self.assertEqual(min('123123'), '1')
         self.assertEqual(min(1, 2, 3), 1)
diff --git a/vm/src/builtins/make_module.rs b/vm/src/builtins/make_module.rs
@@ -508,23 +508,38 @@ mod decl {
     }
 
     #[pyfunction]
-    fn min(args: FuncArgs, vm: &VirtualMachine) -> PyResult {
-        let candidates = match args.args.len().cmp(&1) {
-            std::cmp::Ordering::Greater => args.args.clone(),
-            std::cmp::Ordering::Equal => vm.extract_elements(&args.args[0])?,
+    fn min(mut args: FuncArgs, vm: &VirtualMachine) -> PyResult {
+        let (candidates, default_allowed) = match args.args.len().cmp(&1) {
+            std::cmp::Ordering::Greater => (args.args.clone(), false),
+            std::cmp::Ordering::Equal => (vm.extract_elements(&args.args[0])?, true),
             std::cmp::Ordering::Less => {
                 // zero arguments means type error:
                 return Err(vm.new_type_error("Expected 1 or more arguments".to_owned()));
             }
         };
 
+        let default = args.take_keyword("default");
+        let mut key_func = args.take_keyword("key");
+
+        match args.check_kwargs_empty(vm) {
+            Some(err) => return Err(err),
+            None => {},
+        }
+
+        if !default_allowed && default.is_some() {
+            return Err(vm.new_type_error("Specifying default not allowed with more than 1 argument".to_owned()));
+        }
+
         if candidates.is_empty() {
-            let default = args.get_optional_kwarg("default");
             return default
                 .ok_or_else(|| vm.new_value_error("min() arg is an empty sequence".to_owned()));
         }
 
-        let key_func = args.get_optional_kwarg("key");
+        if let Some(ref obj) = key_func {
+            if vm.is_none(obj) {
+                key_func = None
+            }
+        }
 
         let mut candidates_iter = candidates.into_iter();
         let mut x = candidates_iter.next().unwrap();
diff --git a/vm/src/function.rs b/vm/src/function.rs
@@ -196,12 +196,23 @@ impl FuncArgs {
                 T::arity().end(),
                 given_args,
             )))
-        } else if let Some(k) = self.kwargs.keys().next() {
-            Err(vm.new_type_error(format!("Unexpected keyword argument {}", k)))
-        } else {
+        }  else {
+            match self.check_kwargs_empty(vm) {
+                Some(err) => return Err(err),
+                None => {},
+            }
             Ok(bound)
         }
     }
+
+    pub fn check_kwargs_empty(&self, vm: &VirtualMachine) -> Option<PyBaseExceptionRef> {
+        if let Some(k) = self.kwargs.keys().next() {
+            Some(vm.new_type_error(format!("Unexpected keyword argument {}", k)))
+        } else {
+            None
+        }
+    }
+
 }
 
 /// An error encountered while binding arguments to the parameters of a Python
