Move some methods around

crates/ruff/src/jupyter/notebook.rs

@@ -448,45 +448,40 @@ mod tests {
     use crate::jupyter::{Notebook, NotebookError};
     use crate::registry::Rule;
     use crate::source_kind::SourceKind;
-    use crate::test::{
-        read_jupyter_notebook, test_contents, test_notebook_path, test_resource_path,
-        TestedNotebook,
-    };
+    use crate::test::{test_contents, test_notebook_path, test_resource_path, TestedNotebook};
     use crate::{assert_messages, settings};
 
-    /// Read a Jupyter cell from the `resources/test/fixtures/jupyter/cell` directory.
-    fn read_jupyter_cell(path: impl AsRef<Path>) -> Result<Cell> {
-        let path = test_resource_path("fixtures/jupyter/cell").join(path);
-        let source_code = std::fs::read_to_string(path)?;
-        Ok(serde_json::from_str(&source_code)?)
+    /// Construct a path to a Jupyter notebook in the `resources/test/fixtures/jupyter` directory.
+    fn notebook_path(path: impl AsRef<Path>) -> std::path::PathBuf {
+        test_resource_path("fixtures/jupyter").join(path)
     }
 
     #[test]
-    fn test_valid() {
-        assert!(read_jupyter_notebook(Path::new("valid.ipynb")).is_ok());
+    fn test_python() -> Result<(), NotebookError> {
+        let notebook = Notebook::from_path(&notebook_path("valid.ipynb"))?;
+        assert!(notebook.is_python_notebook());
+        Ok(())
     }
 
     #[test]
-    fn test_r() {
-        // We can load this, it will be filtered out later
-        assert!(read_jupyter_notebook(Path::new("R.ipynb")).is_ok());
+    fn test_r() -> Result<(), NotebookError> {
+        let notebook = Notebook::from_path(&notebook_path("R.ipynb"))?;
+        assert!(!notebook.is_python_notebook());
+        Ok(())
     }
 
     #[test]
     fn test_invalid() {
-        let path = Path::new("resources/test/fixtures/jupyter/invalid_extension.ipynb");
         assert!(matches!(
-            Notebook::from_path(path),
+            Notebook::from_path(&notebook_path("invalid_extension.ipynb")),
             Err(NotebookError::PythonSource(_))
         ));
-        let path = Path::new("resources/test/fixtures/jupyter/not_json.ipynb");
         assert!(matches!(
-            Notebook::from_path(path),
+            Notebook::from_path(&notebook_path("not_json.ipynb")),
             Err(NotebookError::InvalidJson(_))
         ));
-        let path = Path::new("resources/test/fixtures/jupyter/wrong_schema.ipynb");
         assert!(matches!(
-            Notebook::from_path(path),
+            Notebook::from_path(&notebook_path("wrong_schema.ipynb")),
             Err(NotebookError::InvalidSchema(_))
         ));
     }
@@ -497,13 +492,20 @@ mod tests {
     #[test_case(Path::new("only_code.json"), true; "only_code")]
     #[test_case(Path::new("cell_magic.json"), false; "cell_magic")]
     fn test_is_valid_code_cell(path: &Path, expected: bool) -> Result<()> {
+        /// Read a Jupyter cell from the `resources/test/fixtures/jupyter/cell` directory.
+        fn read_jupyter_cell(path: impl AsRef<Path>) -> Result<Cell> {
+            let path = notebook_path("cell").join(path);
+            let source_code = std::fs::read_to_string(path)?;
+            Ok(serde_json::from_str(&source_code)?)
+        }
+
         assert_eq!(read_jupyter_cell(path)?.is_valid_code_cell(), expected);
         Ok(())
     }
 
     #[test]
-    fn test_concat_notebook() -> Result<()> {
-        let notebook = read_jupyter_notebook(Path::new("valid.ipynb"))?;
+    fn test_concat_notebook() -> Result<(), NotebookError> {
+        let notebook = Notebook::from_path(&notebook_path("valid.ipynb"))?;
         assert_eq!(
             notebook.source_code,
             r#"def unused_variable():
@@ -545,77 +547,81 @@ print("after empty cells")
     }
 
     #[test]
-    fn test_import_sorting() -> Result<()> {
-        let path = "isort.ipynb".to_string();
+    fn test_import_sorting() -> Result<(), NotebookError> {
+        let actual = notebook_path("isort.ipynb");
+        let expected = notebook_path("isort_expected.ipynb");
         let TestedNotebook {
             messages,
             source_notebook,
             ..
         } = test_notebook_path(
-            &path,
-            Path::new("isort_expected.ipynb"),
+            &actual,
+            expected,
             &settings::Settings::for_rule(Rule::UnsortedImports),
         )?;
-        assert_messages!(messages, path, source_notebook);
+        assert_messages!(messages, actual, source_notebook);
         Ok(())
     }
 
     #[test]
-    fn test_ipy_escape_command() -> Result<()> {
-        let path = "ipy_escape_command.ipynb".to_string();
+    fn test_ipy_escape_command() -> Result<(), NotebookError> {
+        let actual = notebook_path("ipy_escape_command.ipynb");
+        let expected = notebook_path("ipy_escape_command_expected.ipynb");
         let TestedNotebook {
             messages,
             source_notebook,
             ..
         } = test_notebook_path(
-            &path,
-            Path::new("ipy_escape_command_expected.ipynb"),
+            &actual,
+            expected,
             &settings::Settings::for_rule(Rule::UnusedImport),
         )?;
-        assert_messages!(messages, path, source_notebook);
+        assert_messages!(messages, actual, source_notebook);
         Ok(())
     }
 
     #[test]
-    fn test_unused_variable() -> Result<()> {
-        let path = "unused_variable.ipynb".to_string();
+    fn test_unused_variable() -> Result<(), NotebookError> {
+        let actual = notebook_path("unused_variable.ipynb");
+        let expected = notebook_path("unused_variable_expected.ipynb");
         let TestedNotebook {
             messages,
             source_notebook,
             ..
         } = test_notebook_path(
-            &path,
-            Path::new("unused_variable_expected.ipynb"),
+            &actual,
+            expected,
             &settings::Settings::for_rule(Rule::UnusedVariable),
         )?;
-        assert_messages!(messages, path, source_notebook);
+        assert_messages!(messages, actual, source_notebook);
         Ok(())
     }
 
     #[test]
     fn test_json_consistency() -> Result<()> {
-        let path = "before_fix.ipynb".to_string();
+        let actual_path = notebook_path("before_fix.ipynb");
+        let expected_path = notebook_path("after_fix.ipynb");
+
         let TestedNotebook {
             linted_notebook: fixed_notebook,
             ..
         } = test_notebook_path(
-            path,
-            Path::new("after_fix.ipynb"),
+            actual_path,
+            &expected_path,
             &settings::Settings::for_rule(Rule::UnusedImport),
         )?;
         let mut writer = Vec::new();
         fixed_notebook.write_inner(&mut writer)?;
         let actual = String::from_utf8(writer)?;
-        let expected =
-            std::fs::read_to_string(test_resource_path("fixtures/jupyter/after_fix.ipynb"))?;
+        let expected = std::fs::read_to_string(expected_path)?;
         assert_eq!(actual, expected);
         Ok(())
     }
 
     #[test_case(Path::new("before_fix.ipynb"), true; "trailing_newline")]
     #[test_case(Path::new("no_trailing_newline.ipynb"), false; "no_trailing_newline")]
     fn test_trailing_newline(path: &Path, trailing_newline: bool) -> Result<()> {
-        let notebook = read_jupyter_notebook(path)?;
+        let notebook = Notebook::from_path(&notebook_path(path))?;
         assert_eq!(notebook.trailing_newline, trailing_newline);
 
         let mut writer = Vec::new();
@@ -631,7 +637,7 @@ print("after empty cells")
     // Version 4.5, cell ids are missing and need to be added
     #[test_case(Path::new("add_missing_cell_id.ipynb"), true; "add_missing_cell_id")]
     fn test_cell_id(path: &Path, has_id: bool) -> Result<()> {
-        let source_notebook = read_jupyter_notebook(path)?;
+        let source_notebook = Notebook::from_path(&notebook_path(path))?;
         let source_kind = SourceKind::IpyNotebook(source_notebook);
         let (_, transformed) = test_contents(
             &source_kind,

crates/ruff/src/test.rs

@@ -21,7 +21,7 @@ use ruff_text_size::Ranged;
 
 use crate::autofix::{fix_file, FixResult};
 use crate::directives;
-use crate::jupyter::Notebook;
+use crate::jupyter::{Notebook, NotebookError};
 use crate::linter::{check_path, LinterResult};
 use crate::message::{Emitter, EmitterContext, Message, TextEmitter};
 use crate::packaging::detect_package_root;
@@ -30,18 +30,6 @@ use crate::rules::pycodestyle::rules::syntax_error;
 use crate::settings::{flags, Settings};
 use crate::source_kind::SourceKind;
 
-#[cfg(not(fuzzing))]
-pub(crate) fn read_jupyter_notebook(path: &Path) -> Result<Notebook> {
-    let path = test_resource_path("fixtures/jupyter").join(path);
-    Notebook::from_path(&path).map_err(|err| {
-        anyhow::anyhow!(
-            "Failed to read notebook file `{}`: {:?}",
-            path.display(),
-            err
-        )
-    })
-}
-
 #[cfg(not(fuzzing))]
 pub(crate) fn test_resource_path(path: impl AsRef<Path>) -> std::path::PathBuf {
     Path::new("./resources/test/").join(path)
@@ -67,12 +55,12 @@ pub(crate) fn test_notebook_path(
     path: impl AsRef<Path>,
     expected: impl AsRef<Path>,
     settings: &Settings,
-) -> Result<TestedNotebook> {
-    let source_notebook = read_jupyter_notebook(path.as_ref())?;
+) -> Result<TestedNotebook, NotebookError> {
+    let source_notebook = Notebook::from_path(path.as_ref())?;
 
     let source_kind = SourceKind::IpyNotebook(source_notebook);
     let (messages, transformed) = test_contents(&source_kind, path.as_ref(), settings);
-    let expected_notebook = read_jupyter_notebook(expected.as_ref())?;
+    let expected_notebook = Notebook::from_path(expected.as_ref())?;
     let linted_notebook = transformed.into_owned().expect_ipy_notebook();
 
     assert_eq!(
@@ -273,12 +261,8 @@ Source with applied fixes:
     (messages, transformed)
 }
 
-fn print_diagnostics(
-    diagnostics: Vec<Diagnostic>,
-    file_path: &Path,
-    source: &SourceKind,
-) -> String {
-    let filename = file_path.file_name().unwrap().to_string_lossy();
+fn print_diagnostics(diagnostics: Vec<Diagnostic>, path: &Path, source: &SourceKind) -> String {
+    let filename = path.file_name().unwrap().to_string_lossy();
     let source_file = SourceFileBuilder::new(filename.as_ref(), source.source_code()).finish();
 
     let messages: Vec<_> = diagnostics
@@ -291,15 +275,15 @@ fn print_diagnostics(
         .collect();
 
     if let Some(notebook) = source.notebook() {
-        print_jupyter_messages(&messages, &filename, notebook)
+        print_jupyter_messages(&messages, path, notebook)
     } else {
         print_messages(&messages)
     }
 }
 
 pub(crate) fn print_jupyter_messages(
     messages: &[Message],
-    filename: &str,
+    path: &Path,
     notebook: &Notebook,
 ) -> String {
     let mut output = Vec::new();
@@ -312,7 +296,7 @@ pub(crate) fn print_jupyter_messages(
             &mut output,
             messages,
             &EmitterContext::new(&FxHashMap::from_iter([(
-                filename.to_string(),
+                path.file_name().unwrap().to_string_lossy().to_string(),
                 notebook.clone(),
             )])),
         )