Skip to content

Commit

Permalink
Print missing no-eol in pretty test result
Browse files Browse the repository at this point in the history
Summary:
This CL ensures Scrut suggest to add `(no-eol)` with the pretty renderer if it is missing.

# What?

Append ` (no-eol)` to unexpected output lines that do not end in a new-line in the pretty validation result renderer

# Why?

Before this CL, if a test output validation contained an `equal` rule (i.e. a comparison with a line that ends in a new-line) but expected a `no-eol` rule (i.e. a comparison with a line that does not end with a new-line), then the pretty rendere would provide:

```
1     | - There is no new line
   1  | + There is no new line
```

This was unclear and did not provide the test author with any actionable steps to fix the test. With this CL the output will now correctly show:

```
1     | - There is no new line
   1  | + There is no new line (no-eol)
```

Reviewed By: AndreasBackx

Differential Revision: D63902398

fbshipit-source-id: 0703165dc80b41e1fe32a837493ae320baf5f759
  • Loading branch information
ukautz authored and facebook-github-bot committed Oct 7, 2024
1 parent 2e4558e commit 5cb71b0
Show file tree
Hide file tree
Showing 8 changed files with 111 additions and 3 deletions.
2 changes: 1 addition & 1 deletion py/pytest/parsertest.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def test_fail_on_invalid_exit_code(self) -> None:
)

def test_validation_invalid_input(self) -> None:
result = Output(b"Wrong", b"", 0)
result = Output(b"Wrong\n", b"", 0)
ok, error = self._testcases()[0].validate(result, "the-location")
self.assertEqual(False, ok)
print(ok)
Expand Down
13 changes: 13 additions & 0 deletions selftest/complex/no-eol/invalid.mdtest
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# A test where output does NOT end in new-line

```scrut
$ echo -n There is no new line
There is no new line
```

# A test where output DOES end in new-line

```scrut
$ echo There is a new line
There is a new line (no-eol)
```
36 changes: 36 additions & 0 deletions selftest/complex/no-eol/pretty-renderer-shows-no-eol.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# Test that Scrut proposes noeol if a line does not match for that reason

This test validates that if the user writes a Scrut test where they use an equal-rule (i.e. with ending new-line
character) but the output does not end in a new-line, then Scrut prints the missing `(no-eol)` suffix.

## No-EOL is proposed in the pretty print of the failed test

```scrut
$ "$SCRUT_BIN" test --renderer pretty --match-markdown "*.mdtest" "$TESTDIR/invalid.mdtest"
// =============================================================================
// @ .*[/\\]invalid\.mdtest:4 (regex)
// -----------------------------------------------------------------------------
// # A test where output does NOT end in new-line
// -----------------------------------------------------------------------------
// $ echo -n There is no new line
// =============================================================================
1 | - There is no new line
1 | + There is no new line (no-eol) (equal)
// =============================================================================
// @ .*[/\\]invalid\.mdtest:11 (regex)
// -----------------------------------------------------------------------------
// # A test where output DOES end in new-line
// -----------------------------------------------------------------------------
// $ echo There is a new line
// =============================================================================
1 | - There is a new line (no-eol) (equal)
1 | + There is a new line
Result: 1 file(s) with 2 test(s): 0 succeeded, 2 failed and 0 skipped
[50]
```
22 changes: 22 additions & 0 deletions selftest/complex/no-eol/update-generates-no-eol.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Test that Scrut proposes noeol if a line does not match for that reason

This test validates that if the user writes a Scrut test where they use an equal-rule (i.e. with ending new-line
character) but the output does not end in a new-line, then Scrut prints the missing `(no-eol)` suffix.


```scrut
$ cp "$TESTDIR/invalid.mdtest" "$(pwd)/invalid.mdtest"
```

## No-EOL is proposed in the pretty print of the failed test

```scrut
$ "$SCRUT_BIN" update --match-markdown "*.mdtest" "$(pwd)/invalid.mdtest"
Result: 1 file(s) of which 1 updated, 0 skipped and 0 unchanged
```

## Generated file matches valid result

```scrut
$ diff "$(pwd)/invalid.mdtest.new" "$TESTDIR/valid.mdtest"
```
13 changes: 13 additions & 0 deletions selftest/complex/no-eol/valid.mdtest
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# A test where output does NOT end in new-line

```scrut
$ echo -n There is no new line
There is no new line (no-eol)
```

# A test where output DOES end in new-line

```scrut
$ echo There is a new line
There is a new line
```
10 changes: 9 additions & 1 deletion src/diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use serde::Serialize;

use crate::expectation::Expectation;
use crate::lossy_string;
use crate::newline::BytesNewline;
use crate::newline::SplitLinesByNewline;

/// Compares [`crate::output::Output`]s of [`crate::executors::executor::Executor`]
Expand Down Expand Up @@ -455,7 +456,14 @@ impl Debug for DiffLine {
}
Self::UnexpectedLines { lines } => {
for (index, line) in lines {
write!(f, " {:04} | + {}", index + 1, lossy_string!(line))?;
let eol = (line.as_ref() as &[u8]).ends_in_newline();
write!(
f,
" {:04} | + {}{}",
index + 1,
lossy_string!(line),
if eol { "" } else { " (no-eol)" }
)?;
}
Ok(())
}
Expand Down
7 changes: 7 additions & 0 deletions src/newline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ pub(crate) trait BytesNewline {

/// Returns byte slice that ends in newline character(s)
fn assure_newline(&self) -> Cow<'_, [u8]>;

/// Return bool whether ends in new line
fn ends_in_newline(&self) -> bool;
}

impl BytesNewline for &[u8] {
Expand All @@ -46,6 +49,10 @@ impl BytesNewline for &[u8] {
updated.push(b'\n');
updated.into()
}

fn ends_in_newline(&self) -> bool {
ends_in_newline(self)
}
}

fn ends_in_newline(data: &[u8]) -> bool {
Expand Down
11 changes: 10 additions & 1 deletion src/renderers/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use crate::diff::Diff;
use crate::diff::DiffLine;
use crate::escaping::strip_colors;
use crate::formatln;
use crate::newline::BytesNewline;
use crate::newline::StringNewline;
use crate::outcome::Outcome;
use crate::testcase::TestCaseError;
Expand Down Expand Up @@ -240,9 +241,17 @@ impl ErrorRenderer for PrettyColorRenderer {
}
DiffLine::UnexpectedLines { lines } => {
lines.iter().for_each(|(line_index, line)| {
let eol = (line.as_ref() as &[u8]).ends_in_newline();
let line = if !eol {
let mut line = line.clone();
line.extend(b" (no-eol)");
line
} else {
line.to_owned()
};
let line = outcome
.escaping
.escaped_expectation(line)
.escaped_expectation(&line)
.higlight_tailing_spaces();
last_error_index = Some(diff_index);
output.push_str(
Expand Down

0 comments on commit 5cb71b0

Please sign in to comment.