google / mdbook-i18n-helpers

Translation support for mdbook. The plugins here give you a structured way to maintain a translated book.
Apache License 2.0
126 stars 25 forks source link

`group_events` fuzz test is flaky #150

Closed mgeisler closed 7 months ago

mgeisler commented 7 months ago

After #129, the group_events fuzz test seems to have become flaky. @dalance, would you be able to take a look?

An example failure is here, where the failing input can be minimized to

"````d\n```"

The diff of the failure is

<    "````d\n```\n````",
>    "```d\n```\n```",

meaning that reconstruct_markdown(&events, None) returned

````d
```
````

whereas reconstruct_markdown(&flattened_groups, None) returned

```d
```
```

My guess is that this is because the counting of consequetive ` is slightly off.

Indeed, looking at flattened_groups in the fuzz test, I see

[fuzz_targets/group_events.rs:22:5] &flattened_groups = [
    (
        1,
        Start(
            CodeBlock(
                Fenced(
                    Borrowed(
                        "d",
                    ),
                ),
            ),
        ),
    ),
    (
        2,
        Text(
            Borrowed(
                "`",
            ),
        ),
    ),
    (
        2,
        Text(
            Borrowed(
                "`",
            ),
        ),
    ),

with lots of lone ` characters.

A related question, is this not something which should be fixed in pulldown-cmark-to-cmark instead of here? @dalance, could you create an issue in that repository and see if you can move the fix from here to there?

dalance commented 7 months ago

I've opened a PR to fix the existing token couting code.

I agree pulldown-cmark-to-cmark should construct an appropriate markdown without workaround. I'll consider about it.

mgeisler commented 7 months ago

Awesome, thank you very much!

I see the discussion here: https://github.com/Byron/pulldown-cmark-to-cmark/issues/20#issuecomment-1907767535 and that you've opened https://github.com/Byron/pulldown-cmark-to-cmark/pull/65 to fix it.

Thanks also for #153 — I'm happy to merge that and then simplify our code when the fix has been released in the upstream pulldown-cmark-to-cmark crate.

mgeisler commented 7 months ago

I let it run for a few million iterations on my desktop and it looks rock solid now!

Fixed by #153.