• 
      

    Allow Hosting Service Integration through ReviewBoard Extensions

    Review Request #6572 — Created Nov. 9, 2014 and submitted

    Information

    Review Board
    master
    606121d...

    Reviewers

    There's now a HostingServiceHook that extensions can use to register hosting services. This handles registering, unregistering, and retriving the hosting sevices.

    Unit test have been written to make sure that HostingService subclasses are registered and unregistered properly.

    Description From Last Updated

    This test doesn't assert anything, so what is it testing? If you are expecting something to not throw an exception, …

    brenniebrennie

    Like my response about test_register, this test should check that the hosting service is no longer registered.

    daviddavid
    reviewbot
    1. Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/extensions/hooks.py
      
      
      
      Tool: Pyflakes
      Processed Files:
          reviewboard/extensions/hooks.py
      
      
    2. 
        
    justy777
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          reviewboard/extensions/hooks.py
          reviewboard/extensions/tests.py
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/extensions/hooks.py
          reviewboard/extensions/tests.py
      
      
    2. 
        
    justy777
    brennie
    1. 
        
    2. reviewboard/extensions/tests.py (Diff revision 2)
       
       
      Show all issues

      This test doesn't assert anything, so what is it testing? If you are expecting something to not throw an exception, you should do something like

      try:
          something_that_might_raise()
      except SomeException as e:
          self.fail("Raised SomeException!")
      

      Same for test_unregister.

      1. David advised me that it is not necessary. If an exception is thrown then it causes an error, and the test still fails.

      2. In the case of the sandbox tests, where the purpose of the test is to verify that there are no exceptions, that makes sense. However, this should probably include some assertions that it can find the registered hosting service.

    3. 
        
    david
    1. 
        
    2. reviewboard/extensions/tests.py (Diff revision 2)
       
       
      Show all issues

      Like my response about test_register, this test should check that the hosting service is no longer registered.

    3. 
        
    justy777
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          reviewboard/extensions/hooks.py
          reviewboard/extensions/tests.py
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/extensions/hooks.py
          reviewboard/extensions/tests.py
      
      
    2. 
        
    david
    1. Ship It!
    2. 
        
    justy777
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-2.0.x (784ad96)