Skip to content

test: assert equality in test_byte_roundtrip_signature#22

Merged
dobby-coder[bot] merged 1 commit into
mainfrom
test/signature-roundtrip-assert-eq
May 18, 2026
Merged

test: assert equality in test_byte_roundtrip_signature#22
dobby-coder[bot] merged 1 commit into
mainfrom
test/signature-roundtrip-assert-eq

Conversation

@dobby-coder

@dobby-coder dobby-coder Bot commented May 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds the missing assert_eq!(sig, recovered); in test_byte_roundtrip_signature so it matches its sibling roundtrip tests.

Closes #20.

Test plan

  • cargo test --release --all-features test_byte_roundtrip (5 passed)
  • cargo fmt --all -- --check

Bring test_byte_roundtrip_signature in line with the sibling
roundtrip tests by asserting the decoded Signature equals the
original, catching any future non-canonical decoding regressions.

Closes #20
@dobby-coder

dobby-coder Bot commented May 17, 2026

Copy link
Copy Markdown
Contributor Author

Rule compliance pass: removed the "Signature already derives PartialEq, so the assertion is trivial" bullet from the PR body — that was justification narration on a one-line, self-evident change, which the no-justification-paragraphs-for-simple-changes rule asks to avoid. No code changes.

@dobby-coder dobby-coder Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Self-review (cannot approve own PR): matches sibling roundtrip tests (test_byte_roundtrip_public_key, _secret_key, _user_secret_key) which all assert equality on the decoded value before re-encoding. Signature derives PartialEq at src/gg.rs:90, so the assertion compiles and is meaningful. cargo test --release --all-features test_byte_roundtrip passes all 5; cargo fmt --all -- --check clean. LGTM.

@dobby-coder dobby-coder Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Self-review (cannot self-approve): change is a 1-line test correctness fix aligning test_byte_roundtrip_signature with its four sibling roundtrip tests. Verified locally on test/signature-roundtrip-assert-eq:

  • cargo fmt --all -- --check — clean
  • cargo test --release --all-features test_byte_roundtrip — 5 passed, 0 failed

Signature derives PartialEq (src/gg.rs:60), so the new assert compiles and is meaningful. Closes #20 as described. LGTM pending external approval.

@rubenhensen

Copy link
Copy Markdown
Contributor

/dobby can you merge this pr?

1 similar comment
@rubenhensen

Copy link
Copy Markdown
Contributor

/dobby can you merge this pr?

@dobby-coder

dobby-coder Bot commented May 18, 2026

Copy link
Copy Markdown
Contributor Author

Dobby sees the request! All checks are green, the PR is approved, and Dobby is mergeable and clean — Dobby will press the merge button right away! 🧹

@dobby-coder dobby-coder Bot merged commit b482e51 into main May 18, 2026
8 checks passed
@dobby-coder dobby-coder Bot deleted the test/signature-roundtrip-assert-eq branch May 18, 2026 07:58
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.

test: complete signature roundtrip assertion in test_byte_roundtrip_signature

1 participant