Add new unit tests for djblets.siteconfig

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

Information

Djblets
master

Reviewers

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

    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)
     
     
     
     
     
     
     
     
     
     
    Show all issues

    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)
     
     
    Show all issues
    Col: 71
     E251 unexpected spaces around keyword / parameter equals
    
  3. djblets/siteconfig/tests.py (Diff revision 2)
     
     
    Show all issues
    Col: 73
     E251 unexpected spaces around keyword / parameter equals
    
  4. djblets/siteconfig/tests.py (Diff revision 2)
     
     
    Show all issues
    Col: 34
     E231 missing whitespace after ','
    
  5. djblets/siteconfig/tests.py (Diff revision 2)
     
     
    Show all issues
    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)
     
     
    Show all issues

    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)
     
     
    Show all issues

    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)
     
     
     
     
     
     
     
     
    Show all issues

    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)
     
     
     
     
     
     
     
    Show all issues

    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)
     
     
     
    Show all issues

    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)
     
     
     
     
     
    Show all issues

    You can define this as one statement.

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

    Same here.

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

    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)
     
     
    Show all issues
    Col: 80
     E501 line too long (90 > 79 characters)
    
  3. djblets/siteconfig/tests.py (Diff revision 4)
     
     
    Show all issues
    Col: 80
     E501 line too long (81 > 79 characters)
    
  4. djblets/siteconfig/tests.py (Diff revision 4)
     
     
    Show all issues
    Col: 80
     E501 line too long (81 > 79 characters)
    
  5. djblets/siteconfig/tests.py (Diff revision 4)
     
     
    Show all issues
    Col: 52
     W291 trailing whitespace
    
  6. djblets/siteconfig/tests.py (Diff revision 4)
     
     
    Show all issues
    Col: 26
     E231 missing whitespace after ':'
    
  7. djblets/siteconfig/tests.py (Diff revision 4)
     
     
    Show all issues
    Col: 26
     E231 missing whitespace after ':'
    
  8. djblets/siteconfig/tests.py (Diff revision 4)
     
     
    Show all issues
    Col: 26
     E231 missing whitespace after ':'
    
  9. djblets/siteconfig/tests.py (Diff revision 4)
     
     
    Show all issues
    Col: 80
     E501 line too long (81 > 79 characters)
    
  10. djblets/siteconfig/tests.py (Diff revision 4)
     
     
    Show all issues
    Col: 80
     E501 line too long (85 > 79 characters)
    
  11. djblets/siteconfig/tests.py (Diff revision 4)
     
     
    Show all issues
    Col: 26
     E231 missing whitespace after ':'
    
  12. djblets/siteconfig/tests.py (Diff revision 4)
     
     
    Show all issues
    Col: 26
     E231 missing whitespace after ':'
    
  13. djblets/siteconfig/tests.py (Diff revision 4)
     
     
    Show all issues
    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)
     
     
    Show all issues
    Col: 67
     W291 trailing whitespace
    
  3. djblets/siteconfig/tests.py (Diff revision 5)
     
     
    Show all issues
    Col: 48
     W291 trailing whitespace
    
  4. djblets/siteconfig/tests.py (Diff revision 5)
     
     
    Show all issues
    Col: 48
     W291 trailing whitespace
    
  5. djblets/siteconfig/tests.py (Diff revision 5)
     
     
    Show all issues
    Col: 52
     W291 trailing whitespace
    
  6. djblets/siteconfig/tests.py (Diff revision 5)
     
     
    Show all issues
    Col: 22
     W291 trailing whitespace
    
  7. djblets/siteconfig/tests.py (Diff revision 5)
     
     
    Show all issues
    Col: 48
     W291 trailing whitespace
    
  8. djblets/siteconfig/tests.py (Diff revision 5)
     
     
    Show all issues
    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)
     
     
    Show all issues

    Remove the period. The test runner will add one.

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

    No period.

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

    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)
     
     
    Show all issues

    No period.

  6. djblets/siteconfig/tests.py (Diff revision 6)
     
     
     
     
    Show all issues

    Same note about formatting.

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

    No period.

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

    Same note about formatting.

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

    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)
     
     
    Show all issues
    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:
Completed
Change Summary:
Pushed to release-0.9.x (677381d)