Opened 2 years ago

Last modified 13 months ago

#17306 new enhancement

Split up controller.py's integ tests

Reported by: atagar Owned by: atagar
Priority: Low Milestone:
Component: Core Tor/Stem Version:
Severity: Normal Keywords: testing, easy
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Our integ tests are subdivided by module. Generally this is well and good, but since we're a controller library that means a disproportionate number of tests have... well, been for the controller.

We should further break this up. One thought would be by controller functionality...

  • getinfo commands
  • tor config options (GETCONF, SETCONF)
  • event handling
  • hidden services
  • ... etc...

Child Tickets

Attachments (3)

split_classes.patch (3.3 KB) - added by neel 13 months ago.
Patch to split controller.py integration tests
integ_test.txt (23.0 KB) - added by neel 13 months ago.
Logfile for testing the integration tests
integ_test.2.txt (23.0 KB) - added by neel 13 months ago.
Output of the integration test with the pycodestyle fixes

Download all attachments as: .zip

Change History (7)

comment:1 Changed 13 months ago by neel

Severity: Normal

I have not made all changes listed, but I have split the getinfo commands into separate commands as in this patch:

https://gist.github.com/neelchauhan/2e76169143dc6e24ef69321fcd6d68fb

Let me know if there are any issues with this patch.

Also, the testing log is posted here:

https://gist.github.com/neelchauhan/7b032d05083e85f52d31241a8999700e

Thanks,
Neel Chauhan
===
https://www.neelc.org/

comment:2 Changed 13 months ago by atagar

Thanks Neel! This ticket is pretty old and since then I've had time to think about a good alternate way to structure our tests. Rather than breaking up test functions think we should aim for the test modules so they relate to a single class or topic. That is to say instead of...

class TestController(unittest.TestCase):
  ... tests everything...

... we have...

class TestControllerCommands(unittest.TestCase):
  ... tests GETINFO, GETCONF, etc...

class TestHiddenServices(unittest.TestCase):
  ... tests hidden service functions...

There could be merit to breaking up the functions as you did here too.

Changed 13 months ago by neel

Attachment: split_classes.patch added

Patch to split controller.py integration tests

Changed 13 months ago by neel

Attachment: integ_test.txt added

Logfile for testing the integration tests

comment:3 Changed 13 months ago by neel

I have created new attachments where the classes for the integration test were split. Keep in mind that:

  • This patch does not have the getinfo commands split. I may do it in another patch.
  • In the integration test output, I get control.controller multiple times.

Tell me what you think about this patch.

Thanks,
Neel Chauhan

comment:4 Changed 13 months ago by atagar

Hmmm, now that I see what this looks like maybe this wasn't such a great idea. Sorry about the dead end... :(

One thing in your testing output I'd very much like to correct are all those pyflakes issues. Just pushed a change that should deal with them. Mind giving it a whirl and seeing if you still get any warnings?

Changed 13 months ago by neel

Attachment: integ_test.2.txt added

Output of the integration test with the pycodestyle fixes

Note: See TracTickets for help on using tickets.