Skip to content

Extract test vectors into separate JSON files#33

Merged
jonasnick merged 9 commits intomusig2from
json-vectors
Aug 10, 2022
Merged

Extract test vectors into separate JSON files#33
jonasnick merged 9 commits intomusig2from
json-vectors

Conversation

@robot-dreams
Copy link
Collaborator

This is a re-do of #10 , now that @jonasnick added a bunch of changes and corresponding test cases

It depends on #24, so I'll rebase once that one is merged

Copy link
Owner

@jonasnick jonasnick left a comment

Choose a reason for hiding this comment

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

That's a much better presentation of the test vectors. Concept ACK.

Can you also add an entry for the ChangeLog?

@robot-dreams robot-dreams force-pushed the json-vectors branch 4 times, most recently from 67a7cbf to 1b3da06 Compare July 14, 2022 14:57
@robot-dreams
Copy link
Collaborator Author

This still depends on #24 so shouldn't be merged until that one is.

Copy link
Collaborator

@real-or-random real-or-random left a comment

Choose a reason for hiding this comment

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

Concept ACK

This is really much nicer for other implementations.

Copy link
Collaborator

@real-or-random real-or-random left a comment

Choose a reason for hiding this comment

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

Ok, this is looks nice now. I tested it by playing around with values in the JSON and seeing if the tests catch it.

ACK d005bb2

This helped me in testing, feel free to pick:
real-or-random@70ce59f

ACK 70ce59f

Comment on lines +641 to +643
expected_errors = [
(InvalidContributionError, lambda e: e.signer == 1)
]
Copy link
Owner

Choose a reason for hiding this comment

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

This is unused now

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for catching!

@jonasnick
Copy link
Owner

ACK 8654461

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.

3 participants