[Twisted-Python] Failure of twisted.test.test_plugin.DeveloperSetupTests.test_freshPyReplacesStalePyc under pypy2

[Daniel Sutcliffe]

On Thu, Jul 13, 2017 at 4:22 PM, Daniel Sutcliffe <dansut at gmail.com> 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 <glyph at twistedmatrix.com> 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 <rodrigc at crodrigues.org> 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 <dansut at gmail.com> 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 <dansut at gmail.com>