Skip to content

Enable pypy as a required Travis test#3392

Merged
scoder merged 12 commits intocython:masterfrom
da-woods:pypy_tests
Mar 20, 2020
Merged

Enable pypy as a required Travis test#3392
scoder merged 12 commits intocython:masterfrom
da-woods:pypy_tests

Conversation

@da-woods
Copy link
Contributor

@da-woods da-woods commented Mar 2, 2020

Reasoning being that this make it easier to catch pypy
regressions as they happen.

  • Fixed some very simple pypy failures (largely to do with
    different exception strings)
  • Splits pypy_bugs.txt into four files
    • one for bugs that cause hard crashes (which we don't want to
      run in Travis at all);
    • one for bugs that are probably unfixable because they're just
      due to implementation details (e.g. when destructors are
      called).
    • one file for pypy2-only bugs
    • all other bugs remain in pypy_bugs.txt
      (The categorization has been done fairly quickly, so some bugs
      may be in the wrong place)
  • Made sure (hopefully) all bugs are now categorized, so a basic
    runtests.py with pypy should hopefully pass
  • Changed pypy to be required in Travis
  • Added an extra (optional) test that runs through pypy_bugs.txt.
    The majority of this is expected to fail. This requires an
    extra option to runtest.py "--listfile", which just runs through
    the tests listed in the file.

I haven't made pypy2 a required test in this commit - since
Python2 support is deprecated soon, there seemed limited value in
putting much effort into pypy2.

@da-woods
Copy link
Contributor Author

da-woods commented Mar 2, 2020

I guess the change from optional to required might be controversial, but even if that's the case, hopefully updating the bug list and fixing some of the very easy tests is useful

#define __Pyx_PyDict_GetItemWithError PyDict_GetItemWithError
#define __Pyx_PyDict_GetItemWithError_ErrOccurred PyErr_Occurred
#else
#define __Pyx_PyDict_GetItemWithError PyDict_GetItem
Copy link
Contributor

Choose a reason for hiding this comment

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

That's the thing – this function isn't equivalent to PyDict_GetItemWithError(), because it does not properly report all errors (i.e. exceptions). Thus the actual call to d.get() in Optimize.c, for example. The WithError C-API function was added later for exactly this reason.

Also, such a change definitely shouldn't be part of an "enable pypy in tests" PR. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes that makes sense. I'll get rid of this.

@scoder
Copy link
Contributor

scoder commented Mar 3, 2020

IMHO very worthwhile going in this direction. Would be nice if at least one pypy test job could report actual regressions during development, rather than always being broken.

da-woods added 5 commits March 3, 2020 16:12
Reasoning being that this make it easier to catch pypy3
regressions as they happen.

* Fixed some very simple pypy3 failures (largely to do with
  different exception strings)
* Splits pypy3_bugs.txt into three files
  - one for bugs that cause hard crashes (which we don't want to
    run in Travis at all);
  - one for bugs that are probably unfixable because they're just
    due to implementation details (e.g. when destructors are
    called).
  - all other bugs remain in pypy3_bugs.txt
  (The categorization has been done fairly quickly, so some bugs
  may be in the wrong place)
* Made sure (hopefully) all bugs are now categorized, so a basic
  runtests.py with pypy3 should hopefully pass
* Changed pypy3 to be required in Travis
* Added an extra (optional) test that runs through pypy3_bugs.txt.
  The majority of this is expected to fail. This requires an
  extra option to runtest.py "--listfile", which just runs through
  the tests listed in the file.

I haven't made pypy2 a required test in this commit - since
Python2 support is deprecated soon, there seemed limited value in
putting much effort into pypy2.
Tests weren't failing on my PC because I was using a later version.

I'm not expecting this to fix everything, but I'm hoping this
will make the list of failures more managable to read
Mostly tests that fail on PyPy3 on travis but not on the version
I'm using.

Also did a little bit of re-organizing utilitycode using
PyDict_GetItemWithError to use it in pypy when supported, and
to reduce a little code duplication between python 2 and 3
utilitycode
Undid the changes to Optimize and ModuleSetupCode in favour of
more modest changes.

Added faulthandler to runtests in the hope of being able to
pin-down segmentation faults better on Travis
@da-woods
Copy link
Contributor Author

da-woods commented Mar 3, 2020

I've added tried to use the built-in faulthandler module where available. The hope is that it'll catch segmentation faults a little more clearly on Travis instead of just silently running forever.

@scoder
Copy link
Contributor

scoder commented Mar 5, 2020

Looks good to me so far, just one comment on test inclusion.

da-woods added 2 commits March 5, 2020 12:45
Uses the same mechanism as is used for processing
string passed on commandline
since it seemed pretty close already.

Fixed one text matching issue, and added a specific bugs file
for pypy2
@da-woods da-woods changed the title Enable pypy3 as a required Travis test Enable pypy as a required Travis test Mar 5, 2020
@da-woods
Copy link
Contributor Author

da-woods commented Mar 5, 2020

A number of pypy2 tests are currently having to be excluded because of https://foss.heptapod.net/pypy/pypy/issues/3185. For now I'm just disabling them - I don't see a way round this without a fix from pypy.

@da-woods
Copy link
Contributor Author

da-woods commented Mar 17, 2020

@scoder Can I ping this? Mostly because it'll need re-doing if it sits too long and other PRs break pypy stuff while it's waiting.

@scoder scoder merged commit 9262546 into cython:master Mar 20, 2020
@scoder
Copy link
Contributor

scoder commented Mar 20, 2020

Thanks!

@scoder scoder added this to the 3.0 milestone Mar 20, 2020
@scoder scoder added the Testing label Mar 20, 2020
@da-woods da-woods deleted the pypy_tests branch March 21, 2020 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants