Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(buffer): add assert_buffer_eq! and Debug implementation #161

Merged
merged 1 commit into from
May 7, 2023

Conversation

joshka
Copy link
Member

@joshka joshka commented May 1, 2023

This makes it easier to write unit tests with fast feedback against buffers. The implementation is copied from TestBackend (with some refactoring). Also adds a unit test for buffer_set_string_multi_width_overwrite which was missing from the buffer tests

Output (from some intentionally failing tests):

failures:

---- buffer::tests::buffer_set_string stdout ----
thread 'buffer::tests::buffer_set_string' panicked at 'buffer contents not equal
  expected:
  "baa  "

  actual:
  "aaa  "

  diff:
  0: at (0, 0)
  expected: Cell { symbol: "b", fg: Reset, bg: Reset, modifier: (empty) }
  actual:   Cell { symbol: "a", fg: Reset, bg: Reset, modifier: (empty) }', src/buffer.rs:604:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- buffer::tests::buffer_set_string_zero_width stdout ----
thread 'buffer::tests::buffer_set_string_zero_width' panicked at 'buffer areas not equal
  expected:  Rect { x: 0, y: 0, width: 2, height: 1 }
  actual:    Rect { x: 0, y: 0, width: 1, height: 1 }', src/buffer.rs:642:9

---- buffer::tests::buffer_set_string_multi_width_overwrite stdout ----
thread 'buffer::tests::buffer_set_string_multi_width_overwrite' panicked at 'buffer contents not equal
  expected:
  "称号b" Hidden by multi-width symbols: [(1, " "), (3, " ")]

  actual:
  "称号a" Hidden by multi-width symbols: [(1, " "), (3, " ")]

  diff:
  0: at (4, 0)
  expected: Cell { symbol: "b", fg: Reset, bg: Reset, modifier: (empty) }
  actual:   Cell { symbol: "a", fg: Reset, bg: Reset, modifier: (empty) }', src/buffer.rs:631:9

@joshka
Copy link
Member Author

joshka commented May 1, 2023

@a-kenji would love to hear your thoughts on this approach?

@a-kenji
Copy link
Contributor

a-kenji commented May 1, 2023

It seems that you can't opt in to the cfg(test) feature from outside, meaning I am not able to opt in to using to_test_string() from outside of the crate. Is that correct?

I tried the following:

tui = { git = "https://github.com/joshka/ratatui", branch = "feat-buffer-tests", package = "ratatui" }
terminal.backend().buffer().to_test_string();

Apart from that I like the improvements to the macro.

@joshka
Copy link
Member Author

joshka commented May 2, 2023

It seems that you can't opt in to the cfg(test) feature from outside, meaning I am not able to opt in to using to_test_string() from outside of the crate. Is that correct?

Good point - I wasn't sure about that - perhaps it might be best to just implement a nicer Debug implementation that's useful rather than cell by cell?

@joshka
Copy link
Member Author

joshka commented May 3, 2023

Updated this so that there is a useful implementation of Debug on Buffer and made the assert_buffer_eq! macro use that instead of the to_test_string() method:

failures:

---- buffer::tests::buffer_set_string stdout ----
thread 'buffer::tests::buffer_set_string' panicked at 'buffer contents not equal
expected: Buffer {
    area: Rect { x: 0, y: 0, width: 5, height: 1 },
    content: [
        "aaa  ",
    ],
    styles: [
        x: 0, y: 0, fg: Reset, bg: Reset, modifier: (empty),
    ]
}
actual: Buffer {
    area: Rect { x: 0, y: 0, width: 5, height: 1 },
    content: [
        "baa  ",
    ],
    styles: [
        x: 0, y: 0, fg: Reset, bg: Reset, modifier: (empty),
    ]
}
diff:
0: at (0, 0)
  expected: Cell { symbol: "a", fg: Reset, bg: Reset, modifier: (empty) }
  actual:   Cell { symbol: "b", fg: Reset, bg: Reset, modifier: (empty) }', src/buffer.rs:652:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- buffer::tests::buffer_set_string_multi_width_overwrite stdout ----
thread 'buffer::tests::buffer_set_string_multi_width_overwrite' panicked at 'buffer contents not equal
expected: Buffer {
    area: Rect { x: 0, y: 0, width: 5, height: 1 },
    content: [
        "称号a// hidden by multi-width symbols: [(1, " "), (3, " ")]",
    ],
    styles: [
        x: 0, y: 0, fg: Reset, bg: Reset, modifier: (empty),
    ]
}
actual: Buffer {
    area: Rect { x: 0, y: 0, width: 5, height: 1 },
    content: [
        "称号b// hidden by multi-width symbols: [(1, " "), (3, " ")]",
    ],
    styles: [
        x: 0, y: 0, fg: Reset, bg: Reset, modifier: (empty),
    ]
}
diff:
0: at (4, 0)
  expected: Cell { symbol: "a", fg: Reset, bg: Reset, modifier: (empty) }
  actual:   Cell { symbol: "b", fg: Reset, bg: Reset, modifier: (empty) }', src/buffer.rs:680:9

---- buffer::tests::buffer_set_string_zero_width stdout ----
thread 'buffer::tests::buffer_set_string_zero_width' panicked at 'buffer areas not equal
expected:  Buffer {
    area: Rect { x: 0, y: 0, width: 2, height: 1 },
    content: [
        "a ",
    ],
    styles: [
        x: 0, y: 0, fg: Reset, bg: Reset, modifier: (empty),
    ]
}
actual:    Buffer {
    area: Rect { x: 0, y: 0, width: 1, height: 1 },
    content: [
        "a",
    ],
    styles: [
        x: 0, y: 0, fg: Reset, bg: Reset, modifier: (empty),
    ]
}', src/buffer.rs:691:9

@joshka joshka requested review from sophacles and mindoodoo May 3, 2023 13:39
@joshka joshka changed the title chore: add assert_buffer_eq! macro chore(buffer): impl Debug and add assert_buffer_eq! macro May 3, 2023
@joshka
Copy link
Member Author

joshka commented May 4, 2023

I think this is ready for review

Copy link
Collaborator

@sophacles sophacles left a comment

Choose a reason for hiding this comment

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

Nice - this is great.

Not blocking, but it would be nice to see an #[should_panic] test too, to make sure the macro fails in expected ways

@joshka
Copy link
Member Author

joshka commented May 5, 2023

Not blocking, but it would be nice to see an #[should_panic] test too, to make sure the macro fails in expected ways

Done

@joshka joshka changed the title chore(buffer): impl Debug and add assert_buffer_eq! macro test(buffer): add assert_buffer_eq! and impl Debug May 5, 2023
tests/widgets_block.rs Outdated Show resolved Hide resolved
- The implementation of Debug is customized to make it easy to use the
output (particularly the content) directly when writing tests (by
surrounding it with `Buffer::with_lines(vec![])`). The styles part of
the message shows the position of every style change, rather than the
style of each cell, which reduces the verbosity of the detail, while
still showing everything necessary to debug the buffer.

```rust
Buffer {
    area: Rect { x: 0, y: 0, width: 12, height: 2 },
    content: [
        "Hello World!",
        "G'day World!",
    ],
    styles: [
        x: 0, y: 0, fg: Reset, bg: Reset, modifier: (empty),
        x: 0, y: 1, fg: Green, bg: Yellow, modifier: BOLD,
    ]
}
```

- The assert_buffer_eq! macro shows debug view and diff of the two
buffers, which makes it easy to understand exactly where the difference
is.

- Also adds a unit test for buffer_set_string_multi_width_overwrite
which was missing from the buffer tests
@orhun
Copy link
Sponsor Member

orhun commented May 6, 2023

So the advantage of using this over manual assert_eq! is that it also checks the width and height of the buffer? Is there something that I am missing?

@joshka
Copy link
Member Author

joshka commented May 6, 2023

So the advantage of using this over manual assert_eq! is that it also checks the width and height of the buffer? Is there something that I am missing?

The advantage is in the display when this fails.
Without the debug implementation, the debug format shows the contents cell by cell ([Cell { symbol: "a", style: { fg: color:reset, bg: color:reset, modifier: none }}, Cell { ... }) This makes fixing the issue based on a test failure difficult as you have to look at each symbol individually rather than just seeing (paraphrased)

actual: [
  "aaaaa"
]
expected: [
  "aaaab"
]

When you're writing a test, being able to write a unit test that expects junk "xxxxxx" see that it fails, and then copy the value that the actual call spits out directly into your test makes writing tests super fast (assumes that you check that the actual result is what you intended as part of the testing process). In particularly

The Debug format does most of the heavy lifting on that part, but initially this was just the assert_buffer_eq! macro.

The macro adds the diff, which makes it super obvious where the difference is in cells (again, useful when writing tests for more complex widgets where you have small difficult to notice differences across a large area.

Case in point is #167 which adds a tonne of characterization tests that fully cover the list implementation. Easy with this method, and without it, it would have been much more painful to write easily. Also see #159 (comment):

fujiapple852 May 2, 2023
Yes that would be useful; I wrote a quick'n'dirty one-liner to collect the symbols to a string

Having used this a little bit, I think it's missing an explanation parameter like assert_eq! has (e.g. assert_eq!(1+1, 2, "math is broken {} + {} should = {}", 1, 1, 2);), but I'd prefer to get this in asap rather than being perfect as it's really useful for writing tests.

@orhun orhun changed the title test(buffer): add assert_buffer_eq! and impl Debug test(buffer): add assert_buffer_eq! and Debug implementation May 7, 2023
@orhun orhun merged commit 98b6b19 into ratatui-org:main May 7, 2023
7 checks passed
@joshka joshka deleted the feat-buffer-tests branch July 4, 2023 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants