Skip to content

[Ready for Review] Add msgs serialization tests #292

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

Merged

Conversation

ariard
Copy link

@ariard ariard commented Jan 16, 2019

No description provided.

@ariard ariard changed the title Add bolt7 msgs serialization tests Add msgs serialization tests Jan 16, 2019
Copy link
Collaborator

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

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

Nice. If you're gonna add round-trip serialization tests, why not test all the messages? Also, the fuzz tests probably have reasonable coverage for round-trips, why not test some expected encoded values ala the other tests in msgs?.

src/ln/msgs.rs Outdated
use secp256k1::{Secp256k1, Message};

use rand::{Rng, thread_rng};

Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: extra \n.

src/ln/msgs.rs Outdated

#[test]
fn test_serialization_bolt7_msgs() {

Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: extra \n.

@ariard ariard force-pushed the 2019-12-serialization-test branch from b9f69f1 to 149821f Compare January 19, 2019 03:48
@ariard
Copy link
Author

ariard commented Jan 19, 2019

WIP, more in the way of already there encoding_announcement_signatures

@ariard ariard force-pushed the 2019-12-serialization-test branch 4 times, most recently from eaa82a2 to 282dac8 Compare January 26, 2019 01:01
@ariard
Copy link
Author

ariard commented Jan 26, 2019

Rebased

@ariard ariard force-pushed the 2019-12-serialization-test branch 5 times, most recently from 0272dcd to 931d8b0 Compare January 30, 2019 01:35
@ariard ariard changed the title Add msgs serialization tests [Ready for Review] Add msgs serialization tests Jan 30, 2019
@ariard
Copy link
Author

ariard commented Jan 30, 2019

Good, tests for the 23 types of messages (bolt7 query-thing messages except)

Copy link
Collaborator

@TheBlueMatt TheBlueMatt 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 this! Its probably a bit overkill but it should get us there on coverage.

@ariard ariard force-pushed the 2019-12-serialization-test branch from 931d8b0 to c5d730b Compare February 14, 2019 01:26
@ariard
Copy link
Author

ariard commented Feb 14, 2019

Hmm seems fail is due to travis issue, thread exists forcefully and first time I see these warnings : "Your glibc version:'2.19' will most likely result in malloc()-related deadlocks. Min. version 2.24 (Or, Ubuntu's 2.23-0ubuntu6) suggested. See https://sourceware.org/bugzilla/show_bug.cgi?id=19431 for explanation. Using clone() instead of fork()" ?

@TheBlueMatt TheBlueMatt merged commit 235cf03 into lightningdevkit:master Mar 1, 2019
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