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.

Sign up for GitHub

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

Jump to bottom

lnwire,wtwire: use require package for fuzz tests #7709

Merged
merged 3 commits into from
May 23, 2023
Merged

lnwire,wtwire: use require package for fuzz tests #7709

merged 3 commits into from
May 23, 2023

Conversation

morehouse
Copy link
Collaborator

Simplify code by using the require package instead of t.Fatal.

This is the follow-up PR promised on #7649 (comment). @ellemouton

Tested on my local seed corpora and with a few CPU hours of fuzz time for arbitrary fuzz targets.

@morehouse morehouse changed the title lnwire,wtclient: use require package for fuzz tests lnwire,wtwire: use require package for fuzz tests May 19, 2023
@ellemouton ellemouton self-requested a review May 20, 2023 11:09
guggero
guggero approved these changes May 22, 2023
View reviewed changes
Copy link
Collaborator

@guggero guggero left a comment

Choose a reason for hiding this comment

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

Just a few nits, otherwise LGTM 🎉

lnwire/fuzz_test.go Outdated Show resolved Hide resolved
lnwire/fuzz_test.go Outdated Show resolved Hide resolved
lnwire/fuzz_test.go Show resolved Hide resolved
ellemouton
ellemouton reviewed May 22, 2023
View reviewed changes
Copy link
Collaborator

@ellemouton ellemouton left a comment •
edited

Choose a reason for hiding this comment

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

Looking good! left a simplification suggestion

also - pls remember to add an entry to the release notes :)

lnwire/fuzz_test.go Outdated Show resolved Hide resolved
lnwire/fuzz_test.go Outdated Show resolved Hide resolved
lnwire/fuzz_test.go Outdated
Comment on lines 362 to 364

require.Equal(t, first.ChainHash, second.ChainHash)
require.Equal(t, first.PendingChannelID,
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here as above

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

morehouse added a commit to morehouse/lnd that referenced this pull request May 22, 2023
@morehouse
docs: release note for lightningnetwork#7709
81d7c3e
morehouse added a commit to morehouse/lnd that referenced this pull request May 22, 2023
@morehouse
docs: release note for lightningnetwork#7709
da5000d
ellemouton
ellemouton reviewed May 22, 2023
View reviewed changes
Copy link
Collaborator

@ellemouton ellemouton left a comment

Choose a reason for hiding this comment

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

Thanks for the updates! looking good :) can you pls squash the fixups?

morehouse added a commit to morehouse/lnd that referenced this pull request May 22, 2023
@morehouse
docs: release note for lightningnetwork#7709
41531c7
@morehouse
Copy link
Collaborator Author

Rebased and squashed.

ellemouton
ellemouton approved these changes May 23, 2023
View reviewed changes
Copy link
Collaborator

@ellemouton ellemouton left a comment

Choose a reason for hiding this comment

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

LGTM! Just needs another rebase as there is a merge conflict in the release docs 🙏

@morehouse
lnwire: use require package for fuzz tests
460ba4a 
Simplify code by using the require package instead of t.Fatal().
@morehouse
wtwire: use require package for fuzz tests
4c2e712 
Simplify code by using the require package instead of t.Fatal().
@morehouse
docs: release note for lightningnetwork#7709
c38bb89
@morehouse
Copy link
Collaborator Author

Rebased.

@guggero guggero merged commit 7dacf5e into lightningnetwork:master May 23, 2023
22 of 24 checks passed
@morehouse morehouse deleted the fuzz_require branch May 23, 2023 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@guggero guggero guggero approved these changes

@ellemouton ellemouton ellemouton approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@morehouse @guggero @ellemouton