Add new unit tests for djblets.siteconfig

Review Request #7884 — Created Jan. 16, 2016 and submitted

weijie
Djblets
master
djblets, students
Add new unit tests for djblets.siteconfig

This is a unit test.
I have runned it. All cases are passed.

Description From Last Updated

I suspect this can / should probably be broken up into several smaller test methods on this class - one ...

mike_conleymike_conley

Col: 9 E265 block comment should start with '# '

reviewbotreviewbot

Col: 62 E231 missing whitespace after ','

reviewbotreviewbot

Col: 70 E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

Col: 72 E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

Might as well do: self.assertEqual(self.siteconfig.get('invalidKey', default=2), 2) self.siteconfig.add_default('validKey2', 'validParameter2') self.assertEqual(self.siteconfig.get('validKey2'), 'validParameter2') and so on - that way, you don't need ...

mike_conleymike_conley

Col: 44 E231 missing whitespace after ','

reviewbotreviewbot

Col: 48 E231 missing whitespace after ','

reviewbotreviewbot

Col: 44 E231 missing whitespace after ','

reviewbotreviewbot

Col: 40 E231 missing whitespace after ','

reviewbotreviewbot

Col: 44 E231 missing whitespace after ','

reviewbotreviewbot

Col: 9 E265 block comment should start with '# '

reviewbotreviewbot

Col: 40 E231 missing whitespace after ','

reviewbotreviewbot

Col: 38 E231 missing whitespace after ','

reviewbotreviewbot

Col: 46 E231 missing whitespace after ','

reviewbotreviewbot

Col: 55 E231 missing whitespace after ','

reviewbotreviewbot

Col: 9 E265 block comment should start with '# '

reviewbotreviewbot

Col: 34 E231 missing whitespace after ','

reviewbotreviewbot

Col: 47 E231 missing whitespace after ','

reviewbotreviewbot

Col: 9 E265 block comment should start with '# '

reviewbotreviewbot

Col: 48 E231 missing whitespace after ','

reviewbotreviewbot

Col: 58 E231 missing whitespace after ','

reviewbotreviewbot

Col: 9 E265 block comment should start with '# '

reviewbotreviewbot

Col: 50 E231 missing whitespace after ','

reviewbotreviewbot

Col: 50 E231 missing whitespace after ','

reviewbotreviewbot

Col: 50 E231 missing whitespace after ','

reviewbotreviewbot

Col: 67 W292 no newline at end of file

reviewbotreviewbot

Col: 71 E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

Col: 73 E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

Col: 34 E231 missing whitespace after ','

reviewbotreviewbot

Col: 68 W292 no newline at end of file

reviewbotreviewbot

Each of these tests should have a docstring in the form of: """Testing SiteConfiguration.get with <testing conditions>""" If they need ...

chipx86chipx86

We use under_scores instead of CamelCase. For this, though, I'd just go with a variable name of value, since it's ...

chipx86chipx86

Each thing you're testing should be its own function. That way, if something regresses, we'll see how many of the ...

chipx86chipx86

This would actually be more concise and safer as: self.assertEqual( siteconfig.settings, { 'validKey3': 'validParameter3', 'validKey4': 'validParameter4', })

chipx86chipx86

Blank line between these (between code and blocks, or code and other code doing something different -- think of it ...

chipx86chipx86

You can define this as one statement.

chipx86chipx86

Same here.

chipx86chipx86

This test shouldn't pass because you aren't doing self.siteconfig.add_defaults. Make sure to run the unit test suite (./tests/runtests.py).

brenniebrennie

Col: 80 E501 line too long (90 > 79 characters)

reviewbotreviewbot

Col: 80 E501 line too long (81 > 79 characters)

reviewbotreviewbot

Col: 80 E501 line too long (81 > 79 characters)

reviewbotreviewbot

Col: 52 W291 trailing whitespace

reviewbotreviewbot

Col: 26 E231 missing whitespace after ':'

reviewbotreviewbot

Col: 26 E231 missing whitespace after ':'

reviewbotreviewbot

Col: 26 E231 missing whitespace after ':'

reviewbotreviewbot

Col: 80 E501 line too long (81 > 79 characters)

reviewbotreviewbot

Col: 80 E501 line too long (85 > 79 characters)

reviewbotreviewbot

Col: 26 E231 missing whitespace after ':'

reviewbotreviewbot

Col: 26 E231 missing whitespace after ':'

reviewbotreviewbot

Col: 26 E231 missing whitespace after ':'

reviewbotreviewbot

Col: 67 W291 trailing whitespace

reviewbotreviewbot

Col: 48 W291 trailing whitespace

reviewbotreviewbot

Col: 48 W291 trailing whitespace

reviewbotreviewbot

Col: 52 W291 trailing whitespace

reviewbotreviewbot

Col: 22 W291 trailing whitespace

reviewbotreviewbot

Col: 48 W291 trailing whitespace

reviewbotreviewbot

Col: 48 W291 trailing whitespace

reviewbotreviewbot

Remove the period. The test runner will add one.

brenniebrennie

No period.

brenniebrennie

When the parameters aren't long, you can format as: self.assertEqual(self.siteconfig.get('valid_key_1'), 'valid_parameter_1')

brenniebrennie

No period.

brenniebrennie

Same note about formatting.

brenniebrennie

No period.

brenniebrennie

Same note about formatting.

brenniebrennie

Same not about formatting.

brenniebrennie

Col: 80 E501 line too long (86 > 79 characters)

reviewbotreviewbot
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/siteconfig/tests.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/siteconfig/tests.py
    
    
  2. djblets/siteconfig/tests.py (Diff revision 1)
     
     
    Col: 9
     E265 block comment should start with '# '
    
  3. djblets/siteconfig/tests.py (Diff revision 1)
     
     
    Col: 62
     E231 missing whitespace after ','
    
  4. djblets/siteconfig/tests.py (Diff revision 1)
     
     
    Col: 70
     E251 unexpected spaces around keyword / parameter equals
    
  5. djblets/siteconfig/tests.py (Diff revision 1)
     
     
    Col: 72
     E251 unexpected spaces around keyword / parameter equals
    
  6. djblets/siteconfig/tests.py (Diff revision 1)
     
     
    Col: 44
     E231 missing whitespace after ','
    
  7. djblets/siteconfig/tests.py (Diff revision 1)
     
     
    Col: 48
     E231 missing whitespace after ','
    
  8. djblets/siteconfig/tests.py (Diff revision 1)
     
     
    Col: 44
     E231 missing whitespace after ','
    
  9. djblets/siteconfig/tests.py (Diff revision 1)
     
     
    Col: 40
     E231 missing whitespace after ','
    
  10. djblets/siteconfig/tests.py (Diff revision 1)
     
     
    Col: 44
     E231 missing whitespace after ','
    
  11. djblets/siteconfig/tests.py (Diff revision 1)
     
     
    Col: 9
     E265 block comment should start with '# '
    
  12. djblets/siteconfig/tests.py (Diff revision 1)
     
     
    Col: 40
     E231 missing whitespace after ','
    
  13. djblets/siteconfig/tests.py (Diff revision 1)
     
     
    Col: 38
     E231 missing whitespace after ','
    
  14. djblets/siteconfig/tests.py (Diff revision 1)
     
     
    Col: 46
     E231 missing whitespace after ','
    
  15. djblets/siteconfig/tests.py (Diff revision 1)
     
     
    Col: 55
     E231 missing whitespace after ','
    
  16. djblets/siteconfig/tests.py (Diff revision 1)
     
     
    Col: 9
     E265 block comment should start with '# '
    
  17. djblets/siteconfig/tests.py (Diff revision 1)
     
     
    Col: 34
     E231 missing whitespace after ','
    
  18. djblets/siteconfig/tests.py (Diff revision 1)
     
     
    Col: 47
     E231 missing whitespace after ','
    
  19. djblets/siteconfig/tests.py (Diff revision 1)
     
     
    Col: 9
     E265 block comment should start with '# '
    
  20. djblets/siteconfig/tests.py (Diff revision 1)
     
     
    Col: 48
     E231 missing whitespace after ','
    
  21. djblets/siteconfig/tests.py (Diff revision 1)
     
     
    Col: 58
     E231 missing whitespace after ','
    
  22. djblets/siteconfig/tests.py (Diff revision 1)
     
     
    Col: 9
     E265 block comment should start with '# '
    
  23. djblets/siteconfig/tests.py (Diff revision 1)
     
     
    Col: 50
     E231 missing whitespace after ','
    
  24. djblets/siteconfig/tests.py (Diff revision 1)
     
     
    Col: 50
     E231 missing whitespace after ','
    
  25. djblets/siteconfig/tests.py (Diff revision 1)
     
     
    Col: 50
     E231 missing whitespace after ','
    
  26. djblets/siteconfig/tests.py (Diff revision 1)
     
     
    Col: 67
     W292 no newline at end of file
    
  27. 
      
mike_conley
  1. 
      
  2. djblets/siteconfig/tests.py (Diff revision 1)
     
     

    I suspect this can / should probably be broken up into several smaller test methods on this class - one for each of the sections that you're demarking with a comment.

  3. djblets/siteconfig/tests.py (Diff revision 1)
     
     
     
     
     
     
     
     
     
     

    Might as well do:

    self.assertEqual(self.siteconfig.get('invalidKey', default=2), 2)
    
    self.siteconfig.add_default('validKey2', 'validParameter2')
    self.assertEqual(self.siteconfig.get('validKey2'), 'validParameter2')
    

    and so on - that way, you don't need the intermediate variables.

  4. 
      
WE
WE
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/siteconfig/tests.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/siteconfig/tests.py
    
    
  2. djblets/siteconfig/tests.py (Diff revision 2)
     
     
    Col: 71
     E251 unexpected spaces around keyword / parameter equals
    
  3. djblets/siteconfig/tests.py (Diff revision 2)
     
     
    Col: 73
     E251 unexpected spaces around keyword / parameter equals
    
  4. djblets/siteconfig/tests.py (Diff revision 2)
     
     
    Col: 34
     E231 missing whitespace after ','
    
  5. djblets/siteconfig/tests.py (Diff revision 2)
     
     
    Col: 68
     W292 no newline at end of file
    
  6. 
      
WE
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/siteconfig/tests.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/siteconfig/tests.py
    
    
  2. 
      
chipx86
  1. 
      
  2. djblets/siteconfig/tests.py (Diff revision 3)
     
     

    Each of these tests should have a docstring in the form of:

    """Testing SiteConfiguration.get with <testing conditions>"""
    

    If they need to wrap, do:

    """Testing SiteConfiguration.get with <testing
    conditions>
    """
    
  3. djblets/siteconfig/tests.py (Diff revision 3)
     
     

    We use under_scores instead of CamelCase.

    For this, though, I'd just go with a variable name of value, since it's more clear as to what it is, and short enough to read clearly.

  4. djblets/siteconfig/tests.py (Diff revision 3)
     
     
     
     
     
     
     
     

    Each thing you're testing should be its own function. That way, if something regresses, we'll see how many of the functions failed and have a better sense as to what may have gone wrong.

    Same with the others below.

  5. djblets/siteconfig/tests.py (Diff revision 3)
     
     
     
     
     
     
     

    This would actually be more concise and safer as:

    self.assertEqual(
        siteconfig.settings,
        {
            'validKey3': 'validParameter3',
            'validKey4': 'validParameter4',
        })
    
  6. djblets/siteconfig/tests.py (Diff revision 3)
     
     
     

    Blank line between these (between code and blocks, or code and other code doing something different -- think of it like paragraphs in a story).

  7. djblets/siteconfig/tests.py (Diff revision 3)
     
     
     
     
     

    You can define this as one statement.

  8. djblets/siteconfig/tests.py (Diff revision 3)
     
     
     
     
     

    Same here.

  9. 
      
brennie
  1. 
      
  2. djblets/siteconfig/tests.py (Diff revision 3)
     
     
     
     
     
     
     
     
     
     

    This test shouldn't pass because you aren't doing self.siteconfig.add_defaults.

    Make sure to run the unit test suite (./tests/runtests.py).

  3. 
      
WE
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/siteconfig/tests.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/siteconfig/tests.py
    
    
  2. djblets/siteconfig/tests.py (Diff revision 4)
     
     
    Col: 80
     E501 line too long (90 > 79 characters)
    
  3. djblets/siteconfig/tests.py (Diff revision 4)
     
     
    Col: 80
     E501 line too long (81 > 79 characters)
    
  4. djblets/siteconfig/tests.py (Diff revision 4)
     
     
    Col: 80
     E501 line too long (81 > 79 characters)
    
  5. djblets/siteconfig/tests.py (Diff revision 4)
     
     
    Col: 52
     W291 trailing whitespace
    
  6. djblets/siteconfig/tests.py (Diff revision 4)
     
     
    Col: 26
     E231 missing whitespace after ':'
    
  7. djblets/siteconfig/tests.py (Diff revision 4)
     
     
    Col: 26
     E231 missing whitespace after ':'
    
  8. djblets/siteconfig/tests.py (Diff revision 4)
     
     
    Col: 26
     E231 missing whitespace after ':'
    
  9. djblets/siteconfig/tests.py (Diff revision 4)
     
     
    Col: 80
     E501 line too long (81 > 79 characters)
    
  10. djblets/siteconfig/tests.py (Diff revision 4)
     
     
    Col: 80
     E501 line too long (85 > 79 characters)
    
  11. djblets/siteconfig/tests.py (Diff revision 4)
     
     
    Col: 26
     E231 missing whitespace after ':'
    
  12. djblets/siteconfig/tests.py (Diff revision 4)
     
     
    Col: 26
     E231 missing whitespace after ':'
    
  13. djblets/siteconfig/tests.py (Diff revision 4)
     
     
    Col: 26
     E231 missing whitespace after ':'
    
  14. 
      
WE
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/siteconfig/tests.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/siteconfig/tests.py
    
    
  2. djblets/siteconfig/tests.py (Diff revision 5)
     
     
    Col: 67
     W291 trailing whitespace
    
  3. djblets/siteconfig/tests.py (Diff revision 5)
     
     
    Col: 48
     W291 trailing whitespace
    
  4. djblets/siteconfig/tests.py (Diff revision 5)
     
     
    Col: 48
     W291 trailing whitespace
    
  5. djblets/siteconfig/tests.py (Diff revision 5)
     
     
    Col: 52
     W291 trailing whitespace
    
  6. djblets/siteconfig/tests.py (Diff revision 5)
     
     
    Col: 22
     W291 trailing whitespace
    
  7. djblets/siteconfig/tests.py (Diff revision 5)
     
     
    Col: 48
     W291 trailing whitespace
    
  8. djblets/siteconfig/tests.py (Diff revision 5)
     
     
    Col: 48
     W291 trailing whitespace
    
  9. 
      
WE
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/siteconfig/tests.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/siteconfig/tests.py
    
    
  2. 
      
brennie
  1. This looks great! Just a few formatting nitpicks and then LGTM :)

    You can replace your testing done with:

    Ran unit tests.
    

    That is sufficient for mentioning tests in all patches (unless there are extenuating circumstances).

  2. djblets/siteconfig/tests.py (Diff revision 6)
     
     

    Remove the period. The test runner will add one.

  3. djblets/siteconfig/tests.py (Diff revision 6)
     
     

    No period.

  4. djblets/siteconfig/tests.py (Diff revision 6)
     
     
     
     

    When the parameters aren't long, you can format as:

    self.assertEqual(self.siteconfig.get('valid_key_1'),
                     'valid_parameter_1')
    
  5. djblets/siteconfig/tests.py (Diff revision 6)
     
     

    No period.

  6. djblets/siteconfig/tests.py (Diff revision 6)
     
     
     
     

    Same note about formatting.

  7. djblets/siteconfig/tests.py (Diff revision 6)
     
     

    No period.

  8. djblets/siteconfig/tests.py (Diff revision 6)
     
     
     
     
     
     
     

    Same note about formatting.

  9. djblets/siteconfig/tests.py (Diff revision 6)
     
     
     
     

    Same not about formatting.

  10. 
      
WE
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/siteconfig/tests.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/siteconfig/tests.py
    
    
  2. djblets/siteconfig/tests.py (Diff revision 7)
     
     
    Col: 80
     E501 line too long (86 > 79 characters)
    
  3. 
      
WE
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/siteconfig/tests.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/siteconfig/tests.py
    
    
  2. 
      
WE
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        djblets/siteconfig/tests.py
    
    
    
    Tool: Pyflakes
    Processed Files:
        djblets/siteconfig/tests.py
    
    
  2. 
      
WE
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-0.9.x (677381d)
Loading...