Opened 7 years ago

Closed 6 years ago

#5090 enhancement closed fixed (fixed)

twisted.test.test_udp.MulticastTestCase.test_multiListen implicitly expects multicast group participation to be host-wide not per-socket

Reported by: Glyph Owned by:
Priority: normal Milestone:
Component: core Keywords:
Cc: Branch: branches/multilisten-5090
branch-diff, diff-cov, branch-cov, buildbot
Author: glyph

Description

The test, shown here, has the server join a group but neither client join a group. Then it expects clients to receive messages from that group. This is, of course, silly, and it's a real surprise that it works most of the time. As a bonus, the test's failure mode is to hang forever if it doesn't work.

Change History (7)

comment:1 Changed 7 years ago by Glyph

Owner: set to Glyph
Status: newassigned

Here's a patch. I guess this might even be good enough for review? Assigning it to myself to think about a little more before I commit to that though.

  • twisted/test/test_udp.py

    === modified file 'twisted/test/test_udp.py'
     
    66Tests for implementations of L{IReactorUDP} and L{IReactorMulticast}.
    77"""
    88
    9 from twisted.trial import unittest, util
     9from twisted.trial import unittest
    1010
    1111from twisted.internet.defer import Deferred, gatherResults, maybeDeferred
    1212from twisted.internet import protocol, reactor, error, defer, interfaces, udp
     
    671671        secondPort = reactor.listenMulticast(
    672672            portno, secondClient, listenMultiple=True)
    673673
    674         joined = self.server.transport.joinGroup("225.0.0.250")
     674        theGroup = "225.0.0.250"
     675        joined = gatherResults([self.server.transport.joinGroup(theGroup),
     676                                firstPort.joinGroup(theGroup),
     677                                secondPort.joinGroup(theGroup)])
     678
    675679
    676680        def serverJoined(ignored):
    677681            d1 = firstClient.packetReceived = Deferred()
    678682            d2 = secondClient.packetReceived = Deferred()
    679             firstClient.transport.write("hello world", ("225.0.0.250", portno))
     683            firstClient.transport.write("hello world", (theGroup, portno))
    680684            return gatherResults([d1, d2])
    681685        joined.addCallback(serverJoined)
    682686
     
    698702                                 "processes can listen, but not multiple sockets "
    699703                                 "in same process?")
    700704
     705
    701706if not interfaces.IReactorUDP(reactor, None):
    702707    UDPTestCase.skip = "This reactor does not support UDP"
    703708    ReactorShutdownInteraction.skip = "This reactor does not support UDP"

comment:2 Changed 7 years ago by Jean-Paul Calderone

It'd be nice to turn this into a ReactorBuilder test while you're at it ;)

comment:3 Changed 6 years ago by Glyph

Keywords: review added
Owner: changed from Glyph to Itamar Turner-Trauring
Status: assignednew

comment:4 Changed 6 years ago by Itamar Turner-Trauring

Keywords: review removed
Owner: Itamar Turner-Trauring deleted

Looks good, merge if it is green on all build slaves and you add news file.

comment:5 Changed 6 years ago by Glyph

Author: glyph
Branch: branches/multilisten-5090

(In [32857]) Branching to 'multilisten-5090'

comment:6 Changed 6 years ago by Glyph

Waiting on build results.

comment:7 Changed 6 years ago by Glyph

Resolution: fixed
Status: newclosed

(In [32861]) Fix multicast tests to allow for per-socket rather than per-host groups.

Author: glyph

Reviewer: itamar

Fixes: #5090

twisted.test.test_udp.MulticastTestCase.test_multiListen was implicitly expecting multicast group participation to be host-wide and not per-socket. This change causes all relevant multicast sockets being tested to join the group, instead of relying on the host-wide behavior. In addition to generally being more sensible, this causes the test to no longer hang on systems with this multicast policy.

Note: See TracTickets for help on using tickets.