← Back to team overview

openlp-core team mailing list archive

Re: [Merge] lp:~alisonken1/openlp/ticket-921817 into lp:openlp

 

On Sun, Jan 3, 2016 at 12:41 AM, Tim Bentley <tim.bentley@xxxxxxxxx> wrote:
> Review: Needs Fixing
>

snip

>> +class TestPJLink(TestCase):
>> +    """
>> +    Tests for the PJLink module
>> +    """
>> +    @patch.object(pjlink, 'readyRead')
>> +    @patch.object(pjlink, 'send_command')
>> +    @patch.object(pjlink, 'waitForReadyRead')
>
> Why do I need these 3 here?
> If it is to sep up hidden objects then should they not be created in a # GIVEN:  which is missing
>
> # GIVEN and working pjlink
>
snip

send_command expect an already open network socket to be available.
readyRead and waitForReadyRead are checks that a network socket has
data available to be read.

Since no network socket is available, they are mocked out to avoid
possible test issues with a non-available
network socket.

This test is only to verify that a typo in building the authentication
reply has been fixed.

I believe that other tests I have in mind will require the same setup,
but have a problem trying to get them working in a 'def setUp()'
method.

This test (and future tests I can think of) do not require extra steps
with the mocks, only that they do not attempt to interact with network
sockets that will not be available during test(s).

I can add a # GIVEN: that just points to the previous decorators if
needed - unless there's a better reason for using context managers
rather than decorators for that?

NOTE: Having to relearn a lot of stuff since medical issues I had
several years ago.

-- 
- Ken
Registered Linux user 296561
Slackin' since 1993
Slackware Linux (http://www.slackware.com)

https://code.launchpad.net/~alisonken1/openlp/ticket-921817/+merge/281478
Your team OpenLP Core is subscribed to branch lp:openlp.


References