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.

Sign up for GitHub

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

Jump to bottom

Vector2.scale_to: Add Hypothesis tests, fix exposed bugs #81

Merged
merged 5 commits into from
Dec 9, 2018
Merged

Vector2.scale_to: Add Hypothesis tests, fix exposed bugs #81

merged 5 commits into from
Dec 9, 2018

Conversation

nbraud
Copy link
Collaborator

@nbraud nbraud commented Dec 8, 2018 •
edited
Loading

  • Turned test_scale_is_equivalent_to_truncate into an Hypothesis test.
  • Raise a ZeroDivisionError when scaling a null vector, rather than silently swallowing the error.
  • Raise a ValueError when scaling to a negative length.

nbraud added 2 commits December 8, 2018 21:05
@nbraud
Turn test_scale_is_equivalent_to_truncate into an Hypothesis test
12f6d06
@nbraud
Remove test_calculate_scale
273a9e4 
It was a fairly pointless test, as it used the same code
((scale / x.length) * x) as the actual implementation.
@nbraud
Copy link
Collaborator Author

nbraud commented Dec 8, 2018

@astronouth7303 @pathunstrom The ZeroDivisionError change is potentially breaking (if someone relied on 0 scaling to itself), but swallowing the error results in counter-intuitive behaviour, and I suspect it would be easier for users to have an explicit error there.

nbraud added 3 commits December 8, 2018 21:27
@nbraud
Vector2.scale_to: Add a second property test
65f95f0 
This test reveals two errors:
- the null vector silently fails to scale,
  because scale_to swallows the ZeroDivisionError exception;
- scaling to a negative length should raise a ValueError.
@nbraud
Vector2.scale_to: Raise ZeroDivisionError when scaling a null vector
671e1ae
@nbraud
Vector2.scale_to: Raise ValueError when given a negative length
1b003f2
@nbraud nbraud force-pushed the hypothesis/scale_to branch from 49eee6d to 1b003f2 Compare December 8, 2018 20:28
@nbraud
Copy link
Collaborator Author

nbraud commented Dec 8, 2018

The CI failure seems to be some mypy issue unrelated to this, that I can't reproduce locally.

@AstraLuma
Copy link
Member

re mpy, See python/mypy#6036

@AstraLuma AstraLuma merged commit 513be47 into ppb:master Dec 9, 2018
@nbraud nbraud deleted the hypothesis/scale_to branch December 9, 2018 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@nbraud @AstraLuma