Snapshot report descriptions

panoply commented 2 years ago

Description

Firstly, thank you for creating and maintaining this brilliant tool. I leverage AVA's snapshot feature for comparing responses generated from parsers/lexers and it would be nice if I could feed descriptions of the snapshot test into reports to help better understand what is going in and what I am testing etc.

Currently and AFAIK ava provides this functionality as "labels" when passing a string to the 2nd parameter in the .snapshot() method which will write to the quoted > output, eg:

test('Some example test name', t => {

  const source = '<div>Hello World</div>';

  t.snapshot(source, 'Testing hello world');

})

The above example will output the following in a markdown file:

# Snapshot report for `tests/eg.test.mjs`

The actual snapshot is saved in `eg.mjs.snap`.

Generated by [AVA](https://avajs.dev).

## Some example test name

> Testing hello world

    '<div>Hello World</div>'

While this suffices for a lot of cases, it does not allow for me (and maybe others) to have clear understanding on the purpose of the test and it becomes exceptionally difficult when reviewing snap reports. The current label and test title (even when multi-line) are not enough. Below is an example to better visualize what I mean.

Notice the generate snapshot reports include a description following the test title:

# Snapshot report for `tests/eg.test.mjs`

The actual snapshot is saved in `eg.mjs.snap`.

Generated by [AVA](https://avajs.dev).

## Some example test name

An example of where the description should be output.

> Testing hello world (snapshot 1)

    '<div>Hello World</div>'

> Testing something else (snapshot 2)

    '<div>etc etc etc</div>'

> Testing some else (snapshot 3)

    'xxx xxx xxx xxx'

In the above, a description of the test is applied to the report. Passing markdown descriptions would be ideal but simple multiline strings would also suffice too.

Why you can't use AVA for this

There is no way to apply this (AFAIK) in reports.

And maybe how you think AVA could handle this

Personally, I haven't looked at the overall implementation approach one might employ to support such a capability and I am unsure of how ava is handling this aspect internally. Ideally and given the feature request pertains to snapshot assertions, exposing a description or describe to the t.snapshot method seems elegant and non-intrusive (ie: t.snapshot.describe()). IIRC I once read an issue where someone proposed something similar to be applied on the label parameter but that feels a tad extraneous.

To better demonstrate, see the below example:

_In this I am assuming the following type: t.snapshot.describe(...description: string[]) and also passing some markdown, grain of salt that aspect.


test('Some example test name', t => {

 // ...

  t.snapshot.describe(
    'The snapshot report description can be provided here.',
    'The `describe` method similar to `t.log` could accept spread argument and',
    'maybe some low level **markdown** could be accepted if it does not interfere',
    'with the existing logic, for example:',
    '- List item 1',
    '- List item 2'
    'etc etc',
  )

  t.snapshot(source, 'The snapshot label');

})

The resulting report would be:

# Snapshot report for `tests/eg.test.mjs`

The actual snapshot is saved in `eg.mjs.snap`.

Generated by [AVA](https://avajs.dev).

## Some example test name

The snapshot report description can be provided here. The `describe` method similar to `t.log` could accept spread argument and maybe some low level **markdown** could be accepted if it does not interfere with the existing logic, for example:

- List item 1
- List item 2

etc etc

> The snapshot label

    'xxx xxx xxx xxx'

novemberborn commented 2 years ago

Interesting!

Could perhaps do this as a t.describe() which we could then also include in log output in case the test fails. We'd allow only one call per test, and could perhaps require it to occur before snapshot assertions.

We'd need to add it to the binary snapshot encoding so it's preserved when the Markdown file is re-created when a new test is added.

What do you think?

panoply commented 2 years ago

Could perhaps do this as a t.describe() which we could then also include in log output in case the test fails. We'd allow only one call per test, and could perhaps require it to occur before snapshot assertions.

Sounds utterly delightful. I had a brief look at the existing logic and it seems applicable to employ with not too much refactors or adjustments, though I might be wrong. Extending the log output to support descriptives is also a fantastic idea, but may require some additional thought if markdown is passed, right?

Originally, I wanted to be able to apply descriptions for every snapshot assertion in a test. In my use cases I have multiple snapshot assertions, eg:

test('xxx', t => {

  t.snapshot(source1, 'some label');

  t.snapshot(source2, 'another label');

  t.snapshot(source2, 'yet another label');
})

Being able to inform or better describe each snapshot would be the crème de la crème - meaning markdown reports would be able to generate something like:


# Snapshot report for `tests/eg.test.mjs`

The actual snapshot is saved in `eg.mjs.snap`.

Generated by [AVA](https://avajs.dev).

## Some example test name

An example of where the description should be output.

### some label

Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.

> Testing hello world (snapshot 1)

    '<div>Hello World</div>'

### another label

Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.

> Testing something else (snapshot 2)

    '<div>etc etc etc</div>'

### yet another label

Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.

> Testing some else (snapshot 3)

    'xxx xxx xxx xxx'

However, something of this nature feels a little too much, so I would definitely settle for anything which provides descriptive capabilities in a smart and logical manner (as per your suggestion).

Thanks for the speedy reply and hearing out the suggestion.

novemberborn commented 2 years ago

Extending the log output to support descriptives is also a fantastic idea, but may require some additional thought if markdown is passed, right?

I'd imagine we render it as text, without additional parsing.

Being able to inform or better describe each snapshot would be the crème de la crème

Perhaps the current display style of blockquotes is hampering this? We could do something like:

## Test title

### Snapshot 1

Here's the snapshot message text, it can be Markdown if you like.

    'snapshot'

panoply commented 2 years ago

Also good. For some context, here is an example of snapshot labels I manually augment before assertion. Might give you some idea of the practicality for the descriptives that one can employ.

Perhaps the current display style of blockquotes is hampering this? Yes, indeed.

novemberborn commented 2 years ago

@panoply where do you think we should start? If we change the display style is that enough, or would you also need the t.describe()?

panoply commented 2 years ago

@novemberborn I will fork this week and have a look. I don't want to impede too much on the current approaches. I think a good starting point is changing the display style which is an easy enough refactor (https://github.com/avajs/ava/blob/main/lib/snapshot-manager.js#L88-L101). Extending t.describe() would likely be the most ideal for my use cases but I wouldn't want that to be the driving force of major overhauls.

Lorenzobattistela commented 2 years ago

Is this a good first Issue for a new contributor? I am intersted in contributing with this Issue, are there any material or resources for better understanding of the fixes for this issue? As far as I understand, changes should be made in the blockquotes display style.

If its okay, I am willing to pick up this issue. Thanks!

avajs / ava