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

@knz knz force-pushed the 20221021-simplify-keys branch from c7f520d to 012ddce Compare October 21, 2022 15:10
key.go Outdated Show resolved Hide resolved
@knz knz force-pushed the 20221021-simplify-keys branch 2 times, most recently from 5f79f56 to ce2a006 Compare October 21, 2022 16:14
@knz knz force-pushed the 20221021-simplify-keys branch from ce2a006 to 9e4b2b0 Compare January 7, 2023 19:08
@knz knz requested a review from muesli January 7, 2023 20:21
@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
Contributor

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
Contributor

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 force-pushed the 20221021-simplify-keys branch from 9e4b2b0 to f775c66 Compare June 15, 2023 12:31
@muesli muesli merged commit d9c6751 into charmbracelet:master Jun 15, 2023
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.

2 participants