Craig, thanks for the pointers - I should have known about these really useful resources as I've been lurking a while but for some reason was focussed on the Travis-CI and Appveyor builds. I see now the Twisted Development Wiki page https://twistedmatrix.com/trac/wiki/TwistedDevelopment has links to the buildbot so I have even less excuse for not being more aware. On Wed, Jul 12, 2017 at 3:36 PM, Daniel Sutcliffe wrote:

...

Am I correct in the fact that Twisted doesn't have Travis-CI builds on PyPy?

On Wed, Jul 12, 2017 at 7:31 PM, Craig Rodrigues wrote:

There are two builders which do PyPy:

https://buildbot.twistedmatrix.com/builders/ubuntu16.04-pypy5

Interestingly although there is also an ERROR here for twisted.test.test_plugin.DeveloperSetupTests.test_freshPyReplacesStalePyc it is different to the one I am getting: [ERROR] Traceback (most recent call last): File "/buildslave/ubuntu16.04-pypy5/Twisted/build/pypy-alldeps-nocov-posix/site-packages/twisted/test/test_plugin.py", line 560, in test_freshPyReplacesStalePyc os.utime(pyc.path, (x, x)) exceptions.OSError: [Errno 2] No such file or directory: '/buildslave/ubuntu16.04-pypy5/Twisted/_trial_temp/twisted.test.test_plugin/DeveloperSetupTests/test_freshPyReplacesStalePyc/HBEKNc/temp/application_path/plugindummy/plugins/stale.pyc' Vs mine: [ERROR] Traceback (most recent call last): File "/usr/src/twisted/src/twisted/plugin.py", line 171, in getCache provider = pluginModule.load() File "/usr/src/twisted/src/twisted/python/modules.py", line 392, in load return self.pathEntry.pythonPath.moduleLoader(self.name) File "/usr/src/twisted/src/twisted/python/reflect.py", line 312, in namedAny obj = getattr(obj, n) exceptions.AttributeError: 'module' object has no attribute 'stale' There is also no FAIL in the buildbot build for the same test, mine: [FAIL] Traceback (most recent call last): File "/usr/src/twisted/src/twisted/test/test_plugin.py", line 566, in test_freshPyReplacesStalePyc self.assertIn('one', self.getAllPlugins()) File "/usr/src/twisted/src/twisted/trial/_synctest.py", line 492, in assertIn % (containee, container)) twisted.trial.unittest.FailTest: 'one' not in ['dev', 'app'] These are different from orig report as now in Docker container jamiehewland/alpine-pypy:2 (possibly my goal platform) the original was in a squeaky_pl's portable-pypy 2.7 build of 5.7 pypy. I'm next going to try the official pypy:2 Docker container as this would seem to be possibly the most standard platform to try and support in this day and age... I'm sure I'll get to the bottom of this but any clues or thoughts from the more experienced would be appreciated - it seems to me that there might be some mileage in breaking this problem test up a bit as I gradually understand more about what it is trying to do... ?

https://buildbot.twistedmatrix.com/builders/ubuntu16.04-pypy5-benchmark

and one here: http://speed.twistedmatrix.com/

I had noted this speed comparison when Hawkowl posted it here the other week - should have thought to poke further looking for related builds too as that's what you originally asked about. On a related point; PyPy has entered a strange phase as their 5.8.0 release now is available as CPython 2.7 or 3.5 compatible builds - slightly confusing but I see wheels are tagged something like pp258 pp358 - maybe the Twisted project needs also to somehow differentiate the two. Especially as although the 3.5 builds are still labeled as beta I get the feeling the impetus (sponsorship) is behind removing this designation sooner rather than later. Just a thought

Some of the tests are constantly failing, so if you could investigate and help fix some of the Pypy issues, that would be much appreciated.

I shall do my best as my intention is to use Twisted in a live environment with PyPy so it would be great if the project viewed these builds as required to be clean sometime in the future. This was just the first ERROR I attacked, it was always my intention to look into the others too, which I am also seeing. Cheers /dan -- Daniel Sutcliffe

On Thu, Jul 13, 2017 at 5:57 AM, Daniel Sutcliffe wrote:

...

I shall do my best as my intention is to use Twisted in a live environment with PyPy so it would be great if the project viewed these builds as required to be clean sometime in the future. This was just the first ERROR I attacked, it was always my intention to look into the others too, which I am also seeing.

On Thu, Jul 13, 2017 at 11:31 AM, Craig Rodrigues wrote:

Thanks for your motivation.

No problem, this is one of the few ways I can give back to the Twisted project that is ultimately helping me achieve my goals; besides getting to the bottom of these type of things is a fun learning exercise for me :)

Please keep going, and share your findings on the mailing list.

Last year, Mark Williams worked on fixing many Twisted unit tests to either pass or skip on Pypy, and even found one issue with Pypy itself ( https://bitbucket.org/pypy/pypy/issues/2335/maximum-recursion-depth-exceeded... ).

This one is likely to fall on deaf ears in the PyPy project... The reason for the ERROR and then FAIL is as I originally thought, that __import__() under pypy is not loading .pyc files where the .py is missing and: http://doc.pypy.org/en/latest/config/objspace.lonepycfiles.html tl;dr - they view it as dangerous due to pyc binary variations and only allow by build time configuration option. So the next question I guess is how to deal with this: - presumably my first step is to log a bug in Trac as any change will require one, I had only held off from doing this until I was more sure of the cause of the issue. - the FAIL goes away if the .py is not removed from the plugins dir and it seems to me that the .py removal is not actually required to strictly test what twisted.test.test_plugin.DeveloperSetupTests.test_freshPyReplacesStalePyc purports to be testing - so that could fix this test - the fact that the test ERRORs and then carries on and FAILs later in a strange fashion is due to the fact twisted.plugin.getCache() simply does a log.err() on encountering the odd situation - so maybe this could be dealt with better knowing that PyPy does different things than CPython here... ? Could the more experienced please advise :)

If you feel there are improvements or additions to the buildbot jobs using Pypy, please share your thoughts. The buildbot master.cfg file is at https://github.com/twisted-infra/braid/tree/master/services/buildbot/master

I have not looked into why buildbot comes up with a different error to the one I did on various platforms - fix for my situation and see if it fixes the pypy buildbot before investigating? The other error that I and the buildbot see on pypy is 12x (always) of Failure: twisted.internet.defer.TimeoutError for twisted.protocols.test.test_tls.TLSMemoryBIOTests.test_hugeWrite_TLSv1_1 which is odd as if I run this test on its own it succeeds every time... Thoughts on this appreciated but probably should start a new thread for that... Cheers /dan -- Daniel Sutcliffe

On Thu, Jul 13, 2017 at 4:22 PM, Daniel Sutcliffe wrote:

- the FAIL goes away if the .py is not removed from the plugins dir and it seems to me that the .py removal is not actually required to strictly test what twisted.test.test_plugin.DeveloperSetupTests.test_freshPyReplacesStalePyc purports to be testing - so that could fix this test

OK. Based on your analysis, and also Glyph's recommendation, I think it is OK for this test to skip if Pypy is used. Can you submit a patch for this test to skip on Pypy? I think you can get things rolling if you do something like: (1) Learn the procedure to submit a patch by reading: https://twistedmatrix.com/trac/wiki/TwistedDevelopment#SubmittingaPatch . There are a number of steps, but really it is not that bad. (2) Submit a patch that does the following: - modify twisted/test/test_plugin.py so that you: (a) import _PYPY from twisted.python.compat (b) and then do something like: if _PYPY: test_freshPyReplacesStalePyc.skip = "Does not work on PYPY" I can help review the change to get it in. Thanks! -- Craig

On Thu, Jul 13, 2017 at 4:22 PM, Daniel Sutcliffe wrote:

...

...

- the FAIL goes away if the .py is not removed from the plugins dir and it seems to me that the .py removal is not actually required to strictly test what twisted.test.test_plugin.DeveloperSetupTests.test_freshPyReplacesStalePyc purports to be testing - so that could fix this test

On Thu, Jul 13, 2017 at 10:14 PM Glyph wrote:

...

It is totally fine to skip this test on PyPy. It's testing the ability to interact with a particular feature of the CPython interpreter, which is intentionally absent on PyPy. Ideally, the implementation would not enumerate that module on PyPy since it isn't going to be importable as a module.

On Fri, Jul 14, 2017 at 2:22 AM, Craig Rodrigues wrote:

...

OK. Based on your analysis, and also Glyph's recommendation, I think it is OK for this test to skip if Pypy is used.

Can you submit a patch for this test to skip on Pypy? I think you can get things rolling if you do something like:

(1) Learn the procedure to submit a patch by reading: https://twistedmatrix.com/trac/wiki/TwistedDevelopment#SubmittingaPatch . There are a number of steps, but really it is not that bad.

On Fri, Jul 14, 2017 at 11:47 AM, Daniel Sutcliffe wrote:

Created a ticket to get this ball rolling: https://twistedmatrix.com/trac/ticket/9219 This will be my second PR, but first for actual code, and have been lurking for some while so already have some familiarity with the process - let's see how badly I can screw it up though ;)

PR now exists: https://github.com/twisted/twisted/pull/837

...

(2) Submit a patch that does the following: - modify twisted/test/test_plugin.py so that you: (a) import _PYPY from twisted.python.compat (b) and then do something like:

if _PYPY: test_freshPyReplacesStalePyc.skip = "Does not work on PYPY"

I have an idea for a PR that goes a little bit further than this - I'll submit to see if acceptable and if not I'll fall back to this approach.

I ended up just doing the disabling - after playing around with trying to improve testing in this area I decided that I was getting in over my experience level and thought the basics was a a better first step. I will say though, I think there is some mileage in actually changing the test_plugin.DeveloperSetupTests.test_freshPyReplacesStalePyc to be more of a test_plugin.test_freshPyReplacesCached could remove the need to mess around with the idea of knowing there is the .pyc (with PY3 legacy required) and dropin.cache involved at all and making this test more of a 'unit' test. Then more tests can be created to deal with specific cases, such as test_lonePycLoads can be created (and skipped more meaningfully for PyPy2) ... just a thought, and I don't mind doing if it is thought useful. Cheers /dan -- Daniel Sutcliffe