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

fix(key),test: simplify the input analysis code #568

Merged
merged 1 commit into from Jun 15, 2023

Conversation

knz
Copy link
Contributor

@knz knz commented Oct 21, 2022

This PR contains only the input simplification part of #511.

cc @muesli

key.go Outdated Show resolved Hide resolved
@knz
Copy link
Contributor Author

knz commented Jan 7, 2023

@muesli this is ready again.

@knz knz mentioned this pull request Jan 7, 2023
34 tasks
if m, ok := v.(KeyMsg); ok &&
m.String() != td.out[i].(KeyMsg).String() {
t.Fatalf(`expected a keymsg %q, got %q`, td.out[i].(KeyMsg), m)
// randTest defines the test input and expected output for a sequence
Copy link
Member

Choose a reason for hiding this comment

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

Good enough for now, but I guess we could eventually replace the randomizer with a proper fuzzer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes that is reasonable. Do you want to do this right away? Is there something standard we can use?

Copy link
Member

Choose a reason for hiding this comment

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

Let's keep it as it is, the code looks and works fine for me. As soon as we bump Go to 1.18 we can start moving to the Go fuzzer that's part of the standard toolchain: https://go.dev/security/fuzz/

@muesli muesli merged commit d9c6751 into charmbracelet:master Jun 15, 2023
9 checks passed
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

2 participants