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

group_events fuzz test is flaky #150

Closed
mgeisler opened this issue Jan 16, 2024 · 3 comments · Fixed by #153
Closed

group_events fuzz test is flaky #150

mgeisler opened this issue Jan 16, 2024 · 3 comments · Fixed by #153

Comments

@mgeisler
Copy link
Collaborator

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
Copy link
Contributor

dalance commented Jan 24, 2024

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
Copy link
Collaborator Author

Awesome, thank you very much!

I see the discussion here: Byron/pulldown-cmark-to-cmark#20 (comment) and that you've opened Byron/pulldown-cmark-to-cmark#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 mgeisler linked a pull request Jan 25, 2024 that will close this issue
@mgeisler
Copy link
Collaborator Author

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

Fixed by #153.

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 a pull request may close this issue.

2 participants