Clean up and consolidate avatar unit tests

Review Request #8528 - Created Nov. 7, 2016 and submitted

Barret Rennie
Review Board
release-3.0.x
8526
221c6b9...
reviewboard

The avatar services tests had a few issues, namely:

  • not resetting the registry after test completion;
  • templatetag tests being spread out over two files;
  • some tests not being updated for the new avatar services API; and
  • using improper methods to generate dummy requests.

Some of these issues were causing test failures, while others were more
stylistic issues. All of these issues have been fixed and the avatar
unit tests now all pass.

Ran unit tests.

  • 0
  • 0
  • 10
  • 0
  • 10
Description From Last Updated
Review Bot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/avatars/tests.py
        reviewboard/avatars/templatetags/tests.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/avatars/tests.py
        reviewboard/avatars/templatetags/tests.py
    
    
  2. reviewboard/avatars/tests.py (Diff revision 1)
     
     
     'User' imported but unused
    
  3. reviewboard/avatars/tests.py (Diff revision 1)
     
     
     'HttpRequest' imported but unused
    
  4. reviewboard/avatars/tests.py (Diff revision 1)
     
     
     'Profile' imported but unused
    
  5. reviewboard/avatars/tests.py (Diff revision 1)
     
     
     'avatar_services' imported but unused
    
  6. 
      
Christian Hammond
  1. I think we should have a reviewboard/avatars/tests/, and have test_templatetags.py inside of that, instead of having tests.py in reviewboard/avatars/templatetags/.

    The reason is that, as test suites grow, we tend to split things out into a tests/ directory within the app directory, and all unit tests in there. If we were to do that, but template tag tests lived inside the templatetags/ directory, it wouldn't be run as part of the test suite.

    It also avoids importing tests.py in production, which Django will do when looking for template registries.

  2. 
      
Barret Rennie
David Trowbridge
  1. 
      
  2. reviewboard/avatars/tests.py (Diff revision 2)
     
     
     
     
     
     
     

    Can you put all of this into parens?

  3. reviewboard/avatars/tests.py (Diff revision 2)
     
     
     
     
     
     

    Here too?

  4. reviewboard/avatars/tests.py (Diff revision 2)
     
     
     
     
     
     
     

    And here?

  5. reviewboard/avatars/tests.py (Diff revision 2)
     
     
     
     
     

    And here?

  6. 
      
Barret Rennie
Christian Hammond
  1. 
      
  2. reviewboard/avatars/tests.py (Diff revision 3)
     
     
     
     

    Alphabetical order.

  3. reviewboard/avatars/tests.py (Diff revision 3)
     
     
     
     
     
     
     
     
     
     
     

    Doesn't this leak state that could theoretically impact other tests? Can we instead save/restore values?

    Also, it looks like we're enabling gravatars twice.

    1. Oops, we don't need to reset them on test tear down because we inherit from the AvatarServicesTestMixin

  4. 
      
Barret Rennie
David Trowbridge
  1. Ship It!
  2. 
      
Barret Rennie
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-3.0.x (b682119)
Loading...