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)