gh-102837: Increase test coverage for the math module#110000
gh-102837: Increase test coverage for the math module#110000skirpichev wants to merge 28 commits into
Conversation
* fsum: L1367, L1377, L1381, L1410 // line numbers wrt to 54fbfa8
… nonnegative and PyLong_AsDouble raises only OverflowError.
3dcaaef to
a046568
Compare
September 28, 2023 04:26
serhiy-storchaka
left a comment
There was a problem hiding this comment.
Also, I think that one of changes in your previous PR should be reverted.
Sorry, something went wrong.
Thanks for a suggestion. I did more efficient rewrite of that part, as discussed in the thread. |
Sorry, something went wrong.
|
How did you write new tests? They look wildly random to me. |
Sorry, something went wrong.
I prefer exact results, if possible. Then I try to use arch-independent values (e.g. "big enough" integers to trigger overflows on all platforms. And they should hit mentioned lines. Otherwise, they are random. |
Sorry, something went wrong.
Remove redundant test (python#111416)
|
This PR combines
To make the backporting easier, would it make sense to split the PR along those lines? |
Sorry, something went wrong.
|
@encukou, sorry, I wasn't aware that tests are systematically backported (based on #102067 and #102523).
Probably, you are about typo in sumprod(). This is fixed and backported, see #111342. Or something else? |
Sorry, something went wrong.
|
The backporting is a fairly new policy. But, another benefit of splitting this would be that the test additions could be merged right now, without waiting for @mdickinson to comment on the removed code :) Thanks for the typo fix! |
Sorry, something went wrong.
Sorry, something went wrong.
|
Please could some kind person remove the request for review for me? |
Sorry, something went wrong.
|
@serhiy-storchaka, I'm going to close this (together with the issue) on same ground as #109642. It seems, CPython devs not welcoming now code coverage improvements. |
Sorry, something went wrong.
trunc: drop _PyType_IsReady() check (L2071) like floor/ceilLine numbers are wrt to mathmodule.c at 54fbfa8d5e.
This is a continuation of #102523. There are still some uncovered lines/branches, so issue will not be closed. I can add more commits, but I feel it will be more difficult to review...