Opened 4 years ago

Closed 4 years ago

#6332 enhancement closed fixed (fixed)

fake twisted.trial._dist.test.test_workertrial.MainTestCase.fdopen returns None sometimes

Reported by: Glyph Owned by: therve
Priority: normal Milestone:
Component: core Keywords:
Cc: Branch: branches/dist-fdopen-6332
branch-diff, diff-cov, branch-cov, buildbot
Author: therve

Description

On a recent ticket related to a Python regression, this makes some of the tracebacks harder to understand, specifically, this traceback:

[ERROR]
Traceback (most recent call last):
  File "/var/lib/buildbot/bot-glyph-1/lucid32-py2.7maint/Twisted/twisted/plugin.py", line 181, in getCache
    dropinPath.setContent(pickle.dumps(dropinDotCache))
  File "/var/lib/buildbot/bot-glyph-1/lucid32-py2.7maint/Twisted/twisted/python/filepath.py", line 1343, in setContent
    f.close()
exceptions.AttributeError: 'NoneType' object has no attribute 'close'

twisted.trial._dist.test.test_workertrial.MainTestCase.test_empty
twisted.trial._dist.test.test_workertrial.MainTestCase.test_empty
twisted.trial._dist.test.test_workertrial.MainTestCase.test_otherReadError
twisted.trial._dist.test.test_workertrial.MainTestCase.test_otherReadError
twisted.trial._dist.test.test_workertrial.MainTestCase.test_readInterrupted
twisted.trial._dist.test.test_workertrial.MainTestCase.test_readInterrupted

This is a bogus error, apparently occurring because the fd value is something that we weren't expecting. Since '3' and '4' are not standard file descriptor values (the only "special" ones are 0, 1, 2) this fixture should probably be a little more resilient. Even if it remains as-is, it should fail earlier and in a more coherent way if a non-supported FD is passed in; it's not valid for fdopen to return None.

Change History (5)

comment:1 Changed 4 years ago by therve

Owner: set to therve

comment:2 Changed 4 years ago by therve

Author: therve
Branch: branches/dist-fdopen-6332

(In [37257]) Branching to 'dist-fdopen-6332'

comment:3 Changed 4 years ago by therve

Keywords: review added
Owner: therve deleted

Yeah that was dumb. A better version is in the branch, not faking the whole module and being a bit more clear about the intent.

comment:4 Changed 4 years ago by Tom Prince

Keywords: review removed
Owner: set to therve
  1. The fdopen argument to main needs to be documented, and should perhaps be private.
  2. While reading the tests, I noticed test_readInterrupted still passes if the continue is changed to return in workertrial.main. There should be a ticket opened to deal with this.
  3. There was a failure on the buildbot, which I can't seem to reproduce. (I guess this is unrelated)

Please commit after addressing 1+2 and running on buildbot.

comment:5 Changed 4 years ago by therve

Resolution: fixed
Status: newclosed

(In [37270]) Merge dist-fdopen-6332

Author: therve Reviewer: tom.prince Fixes: #6332

Remove patch fdopen from test_workertrial and parametrize the tested function instead, fixing tracebacks with 2.7 maintenance branch.

Note: See TracTickets for help on using tickets.