Skip to content

Commit

Permalink
Add test for Notebook text output (#7925)
Browse files Browse the repository at this point in the history
## Summary

This PR adds test cases for the Notebook output in text format.

## Test Plan

Update test snapshots.
  • Loading branch information
dhruvmanila authored Oct 13, 2023
1 parent cd564c4 commit f08a5f6
Show file tree
Hide file tree
Showing 5 changed files with 177 additions and 2 deletions.
1 change: 1 addition & 0 deletions crates/ruff_linter/src/message/diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ impl<'a> Diff<'a> {

impl Display for Diff<'_> {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
// TODO(dhruvmanila): Add support for Notebook cells once it's user-facing
let mut output = String::with_capacity(self.source_code.source_text().len());
let mut last_end = TextSize::default();

Expand Down
122 changes: 121 additions & 1 deletion crates/ruff_linter/src/message/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,8 @@ mod tests {
use rustc_hash::FxHashMap;

use ruff_diagnostics::{Diagnostic, DiagnosticKind, Edit, Fix};
use ruff_source_file::SourceFileBuilder;
use ruff_notebook::NotebookIndex;
use ruff_source_file::{OneIndexed, SourceFileBuilder};
use ruff_text_size::{Ranged, TextRange, TextSize};

use crate::message::{Emitter, EmitterContext, Message};
Expand Down Expand Up @@ -221,6 +222,113 @@ def fibonacci(n):
]
}

pub(super) fn create_notebook_messages() -> (Vec<Message>, FxHashMap<String, NotebookIndex>) {
let notebook = r"# cell 1
import os
# cell 2
import math
print('hello world')
# cell 3
def foo():
print()
x = 1
";

let unused_import_os = Diagnostic::new(
DiagnosticKind {
name: "UnusedImport".to_string(),
body: "`os` imported but unused".to_string(),
suggestion: Some("Remove unused import: `os`".to_string()),
},
TextRange::new(TextSize::from(16), TextSize::from(18)),
)
.with_fix(Fix::safe_edit(Edit::range_deletion(TextRange::new(
TextSize::from(9),
TextSize::from(19),
))));

let unused_import_math = Diagnostic::new(
DiagnosticKind {
name: "UnusedImport".to_string(),
body: "`math` imported but unused".to_string(),
suggestion: Some("Remove unused import: `math`".to_string()),
},
TextRange::new(TextSize::from(35), TextSize::from(39)),
)
.with_fix(Fix::safe_edit(Edit::range_deletion(TextRange::new(
TextSize::from(28),
TextSize::from(40),
))));

let unused_variable = Diagnostic::new(
DiagnosticKind {
name: "UnusedVariable".to_string(),
body: "Local variable `x` is assigned to but never used".to_string(),
suggestion: Some("Remove assignment to unused variable `x`".to_string()),
},
TextRange::new(TextSize::from(98), TextSize::from(99)),
)
.with_fix(Fix::unsafe_edit(Edit::deletion(
TextSize::from(94),
TextSize::from(104),
)));

let notebook_source = SourceFileBuilder::new("notebook.ipynb", notebook).finish();

let mut notebook_indexes = FxHashMap::default();
notebook_indexes.insert(
"notebook.ipynb".to_string(),
NotebookIndex::new(
vec![
OneIndexed::from_zero_indexed(0),
OneIndexed::from_zero_indexed(0),
OneIndexed::from_zero_indexed(1),
OneIndexed::from_zero_indexed(1),
OneIndexed::from_zero_indexed(1),
OneIndexed::from_zero_indexed(1),
OneIndexed::from_zero_indexed(2),
OneIndexed::from_zero_indexed(2),
OneIndexed::from_zero_indexed(2),
OneIndexed::from_zero_indexed(2),
],
vec![
OneIndexed::from_zero_indexed(0),
OneIndexed::from_zero_indexed(1),
OneIndexed::from_zero_indexed(0),
OneIndexed::from_zero_indexed(1),
OneIndexed::from_zero_indexed(2),
OneIndexed::from_zero_indexed(3),
OneIndexed::from_zero_indexed(0),
OneIndexed::from_zero_indexed(1),
OneIndexed::from_zero_indexed(2),
OneIndexed::from_zero_indexed(3),
],
),
);

let unused_import_os_start = unused_import_os.start();
let unused_import_math_start = unused_import_math.start();
let unused_variable_start = unused_variable.start();

(
vec![
Message::from_diagnostic(
unused_import_os,
notebook_source.clone(),
unused_import_os_start,
),
Message::from_diagnostic(
unused_import_math,
notebook_source.clone(),
unused_import_math_start,
),
Message::from_diagnostic(unused_variable, notebook_source, unused_variable_start),
],
notebook_indexes,
)
}

pub(super) fn capture_emitter_output(
emitter: &mut dyn Emitter,
messages: &[Message],
Expand All @@ -232,4 +340,16 @@ def fibonacci(n):

String::from_utf8(output).expect("Output to be valid UTF-8")
}

pub(super) fn capture_emitter_notebook_output(
emitter: &mut dyn Emitter,
messages: &[Message],
notebook_indexes: &FxHashMap<String, NotebookIndex>,
) -> String {
let context = EmitterContext::new(notebook_indexes);
let mut output: Vec<u8> = Vec::new();
emitter.emit(&mut output, messages, &context).unwrap();

String::from_utf8(output).expect("Output to be valid UTF-8")
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
---
source: crates/ruff_linter/src/message/text.rs
expression: content
---
notebook.ipynb:cell 1:2:8: F401 [*] `os` imported but unused
|
1 | # cell 1
2 | import os
| ^^ F401
|
= help: Remove unused import: `os`

notebook.ipynb:cell 2:2:8: F401 [*] `math` imported but unused
|
1 | # cell 2
2 | import math
| ^^^^ F401
3 |
4 | print('hello world')
|
= help: Remove unused import: `math`

notebook.ipynb:cell 3:4:5: F841 [*] Local variable `x` is assigned to but never used
|
2 | def foo():
3 | print()
4 | x = 1
| ^ F841
|
= help: Remove assignment to unused variable `x`


17 changes: 16 additions & 1 deletion crates/ruff_linter/src/message/text.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,10 @@ struct SourceCode<'a> {
mod tests {
use insta::assert_snapshot;

use crate::message::tests::{capture_emitter_output, create_messages};
use crate::message::tests::{
capture_emitter_notebook_output, capture_emitter_output, create_messages,
create_notebook_messages,
};
use crate::message::TextEmitter;
use crate::settings::types::UnsafeFixes;

Expand Down Expand Up @@ -383,4 +386,16 @@ mod tests {

assert_snapshot!(content);
}

#[test]
fn notebook_output() {
let mut emitter = TextEmitter::default()
.with_show_fix_status(true)
.with_show_source(true)
.with_unsafe_fixes(UnsafeFixes::Enabled);
let (messages, notebook_indexes) = create_notebook_messages();
let content = capture_emitter_notebook_output(&mut emitter, &messages, &notebook_indexes);

assert_snapshot!(content);
}
}
7 changes: 7 additions & 0 deletions crates/ruff_notebook/src/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@ pub struct NotebookIndex {
}

impl NotebookIndex {
pub fn new(row_to_cell: Vec<OneIndexed>, row_to_row_in_cell: Vec<OneIndexed>) -> Self {
Self {
row_to_cell,
row_to_row_in_cell,
}
}

/// Returns the cell number (1-based) for the given row (1-based).
pub fn cell(&self, row: OneIndexed) -> Option<OneIndexed> {
self.row_to_cell.get(row.to_zero_indexed()).copied()
Expand Down

0 comments on commit f08a5f6

Please sign in to comment.