Opened 6 years ago

Closed 6 years ago

#3113 enhancement closed fixed (fixed)

Make SSHProtocolTestCase easier to customize/reuse by another tests.

Reported by: jkk Owned by: jkk
Priority: normal Milestone:
Component: conch Keywords:
Cc: Branch:
Author: Launchpad Bug:

Description

I've broken testOurServerOurClient into two other methods.One responible for server/client initiializiation and another for running connection between them.
This allows one to customize server or client side classes without code duplication.

My use case is adding test for #3007.I would use testOurServerOurClient, but I need to
use custom client class different from ConchTestClient().I can't do that without duplicating the code.With patch I would create a new method in SSHProtcolTestCase or it's subclass and change self.client.

Attachments (2)

conch-test_ssh-SSHProtocolTestCase.patch (1.2 KB) - added by jkk 6 years ago.
test_ssh-refactor.patch (1.3 KB) - added by exarkun 6 years ago.

Download all attachments as: .zip

Change History (5)

comment:1 Changed 6 years ago by exarkun

  • Owner changed from z3p to exarkun
  • Status changed from new to assigned

comment:2 Changed 6 years ago by exarkun

  • Keywords review removed
  • Owner changed from exarkun to jkk
  • Status changed from assigned to new

This seems like a fine refactoring to do. I've made a few changes to the patch to bring it in line with the coding standard (docstrings, whitespace, test method naming), but otherwise it looks good. However, this is such a small change I don't think it makes sense to do it separately from #3007. I suggest just including it in whatever patch you submit to resolve that ticket.

I'll attach the modified patch so you can use that.

If this makes sense to you, please close this ticket.

Changed 6 years ago by exarkun

comment:3 Changed 6 years ago by jkk

  • Resolution set to fixed
  • Status changed from new to closed

Thanks for a really quick response and feedback :) I'll use it in #3007.

Note: See TracTickets for help on using tickets.