-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add Hypothesis tests for TradeCodes #71
Add Hypothesis tests for TradeCodes #71
Conversation
eaefc8d
to
d87f9d4
Compare
c8ab8cc
to
9010d5b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few comments and some suggestions for improvements.
c233658
to
fbf8141
Compare
abd6673
to
2285861
Compare
9d8b599
to
c9591fc
Compare
c9591fc
to
da77572
Compare
da77572
to
12bb779
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's get this merged in. I'm concerned with the _process_sophonts_and_homeworlds()
being a hand-written parser and therefore fragile. But let's fix that in the next pass.
This PR hooks up the Hypothesis testing library, which enables property-based tests:
Starline parsing has historically been less than robust, and trade code parsing was no better than the rest of it, so this PR started there.
The specification i used was
Given an otherwise-valid input string, it should parse cleanly to a well-formed TradeCodes object that then should cleanly round-trip to/from string
. As could be expected, smoke poured out of TradeCode parsing. So I went thru and fixed that.