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

added proptest & impl Arbitrary traits for quote #2

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Leechael
Copy link

@Leechael Leechael commented Oct 18, 2024

Added the Arbitrary trait to each Quote struct to simplify mocking and testing.

@@ -36,13 +51,89 @@ pub struct Header {
pub user_data: [u8; 20],
}

#[derive(Decode, Debug)]
impl Arbitrary for Header {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Code readability suffers when implementing traits manually like this, and it's error-prone. Consider using #[derive(Arbitrary)] instead.

T: Encode + TryFrom<usize>,
<T as TryFrom<usize>>::Error: std::fmt::Debug,
{
fn encode(&self) -> Vec<u8> {
Copy link
Collaborator

@kvinwang kvinwang Oct 25, 2024

Choose a reason for hiding this comment

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

Prefer implementing fn encode_to rather than fn encode to avoid one extra heap allocation + data copying.

@@ -181,6 +529,16 @@ impl Decode for AuthDataV4 {
}
}

impl Encode for AuthDataV4 {
fn encode(&self) -> Vec<u8> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Prefer encode_to

Comment on lines +23 to +24
let mut encoded = vec![];
header.encode_to(&mut encoded);
Copy link
Collaborator

@kvinwang kvinwang Oct 25, 2024

Choose a reason for hiding this comment

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

Suggested change
let mut encoded = vec![];
header.encode_to(&mut encoded);
let encoded = header.encode();

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