fix: Ensure `pow(3, -4, 5)` throws ValueError

beeware / batavia

A JavaScript implementation of the Python virtual machine.

http://pybee.org/batavia

Other

1.39k stars 424 forks source link

fix: Ensure `pow(3, -4, 5)` throws ValueError #746

Closed cLupus closed 5 years ago

cLupus commented 6 years ago

pow(x, y, z) throw a ValueError when y is negative, while x, and ,z are positive.

Additionally cleaned up the javascript with ECMAscript 6 recommendations; i.e. the use of let, or const instead of var, and triple = instead of double.

Related issue: #46

PR Checklist:

[x] All new features have been tested
[ ] All new features have been documented
[x] I have read the CONTRIBUTING.md file
[x] I will abide by the code of conduct

phildini commented 6 years ago

Hi there! Did this fix any tests? If not, should tests be added to prove the fix works?

cLupus commented 6 years ago

Do you have a suggestion on how to write a test for that?

Note that, the previous two tests (pow(3, -4, 5), and some other pow with a negative power, and a modulo) failed before this PR.

shaib commented 6 years ago

Well, I can't seem to be able to approve the PR, but I can verify that when I run the tests on Python 3.6, there are two failing tests in tests.builtins.test_pow.PowTests -- test_int_neg_y_pos_z and test_int_neg_y_neg_z -- which fail without the PR, and pass with it; and as far as I can see, this is exactly what the PR set out to fix.

freakboy3742 commented 6 years ago

@shaib Hrm - that's odd - any idea why those tests aren't failing on CI?

martica commented 5 years ago

Thanks for the PR, @cLupus! It looks like this got covered by another PR, so I'm going to close this out.