WIP Added testing skeleton for the reviewbotext directory

Review Request #11237 — Created Oct. 21, 2020 and updated

jace
ReviewBot
master
b0dbd60...
reviewbot
ceciliawei, jblazusi

Fixed comment length (changed to 70 characters)

Added .nosids and .test_notes to .gitignore (Note: I'm hoping I did it properly. I may have only added the file name and not a path so please let me know if I messed up).

Moved all subclasses from the previous runtests.py file into their own files.

Added more empty function definitions in the new files that match the tests I have planned for that file.

Also, I'm assuming I'm going to have flake8 errors again. I'm not exactly sure how to execute "flake8 --ignore E121,E125,E129,E241 --max-line-length 79 PATH" locally as it gives me an error in my terminal. If anyone knows what I'm doing wrong please let me know.



Description From Last Updated

Can you wrap the description to ~70 (upwards of 75) characters per row? That's the standard for the description wrapping. ...

ceciliaweiceciliawei

This file is auto-generated and shouldn't be part of your commit. Let's remove it from this and instead add .noseids ...

daviddavid

Rather than call this runtests.py and have this shebang (which indicates that it should be run as a script), let's ...

daviddavid

The top-level module name is reviewbotext (ReviewBot and extension are just directories in the source). So these should be: from ...

daviddavid

E302 expected 2 blank lines, found 1

reviewbotreviewbot

E999 SyntaxError: invalid syntax

reviewbotreviewbot

E302 expected 2 blank lines, found 1

reviewbotreviewbot

E302 expected 2 blank lines, found 1

reviewbotreviewbot

E302 expected 2 blank lines, found 1

reviewbotreviewbot

E302 expected 2 blank lines, found 1

reviewbotreviewbot

E302 expected 2 blank lines, found 1

reviewbotreviewbot

E302 expected 2 blank lines, found 1

reviewbotreviewbot

E302 expected 2 blank lines, found 1

reviewbotreviewbot

E302 expected 2 blank lines, found 0

reviewbotreviewbot

E302 expected 2 blank lines, found 1

reviewbotreviewbot

E302 expected 2 blank lines, found 1

reviewbotreviewbot

E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

W191 indentation contains tabs

reviewbotreviewbot

E999 IndentationError: unexpected indent

reviewbotreviewbot

E113 unexpected indentation

reviewbotreviewbot

E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

E115 expected an indented block (comment)

reviewbotreviewbot

E112 expected an indented block

reviewbotreviewbot

E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

W191 indentation contains tabs

reviewbotreviewbot

E303 too many blank lines (2)

reviewbotreviewbot

W191 indentation contains tabs

reviewbotreviewbot

W191 indentation contains tabs

reviewbotreviewbot

E303 too many blank lines (2)

reviewbotreviewbot

W191 indentation contains tabs

reviewbotreviewbot

W191 indentation contains tabs

reviewbotreviewbot

E303 too many blank lines (2)

reviewbotreviewbot

W191 indentation contains tabs

reviewbotreviewbot

W191 indentation contains tabs

reviewbotreviewbot

E303 too many blank lines (2)

reviewbotreviewbot

W191 indentation contains tabs

reviewbotreviewbot

W191 indentation contains tabs

reviewbotreviewbot

E303 too many blank lines (2)

reviewbotreviewbot

W191 indentation contains tabs

reviewbotreviewbot

W191 indentation contains tabs

reviewbotreviewbot

W191 indentation contains tabs

reviewbotreviewbot

E265 block comment should start with '# '

reviewbotreviewbot

E302 expected 2 blank lines, found 1

reviewbotreviewbot

E999 SyntaxError: invalid syntax

reviewbotreviewbot

E101 indentation contains mixed spaces and tabs

reviewbotreviewbot
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

jace
ceciliawei
  1. 
      
    • Can you wrap the description to ~70 (upwards of 75) characters per row? That's the standard for the description wrapping.
    • I noticed you are having some flake8 errors. You can check locally using flake8 --ignore E121,E125,E129,E241 --max-line-length 79 PATH before publishing the review. That should save you some time.
  2. 
      
david
  1. 
      
  2. extension/tests/.noseids (Diff revision 1)
     
     

    This file is auto-generated and shouldn't be part of your commit. Let's remove it from this and instead add .noseids to the top-level .gitignore file.

  3. extension/tests/runtests.py (Diff revision 1)
     
     

    Rather than call this runtests.py and have this shebang (which indicates that it should be run as a script), let's just have a tests/ directory with a handful of files in it for each major section, like tests/test_integration.py, tests/test_resources.py, etc. Each of those files will be auto-discovered by the test runner.

  4. extension/tests/runtests.py (Diff revision 1)
     
     
     

    The top-level module name is reviewbotext (ReviewBot and extension are just directories in the source). So these should be:

    from reviewbotext.extension import ReviewBotExtension
    from reviewbotext.integration import ReviewBotIntegration
    
  5. 
      
jblazusi
  1. I noticed that your methods are defined as def setUP(self), however, I think that PEP 8, which is largely what Review Board follows uses function names with lowercase letters and underscores to separate, rather than camelCase. Correct me if I am wrong, but I think it should be def set_up(self)

    1. This is because it's overriding the built-in UnitTest.setUp method. This one doesn't follow the standards, and for some reason in Python 3, when they had the opportunity to fix it, they chose not to. So it will always be inconsistent.

    2. Err, TestCase.setUp, not UnitTest.

    3. That is a huge bummer. In that case Jace, you can ignore my comments about that.

  2. 
      
jace
Review request changed

Change Summary:

[WIP] Implemented review feedback and expanded on structure of the testing suite (this is the post that I messed up last week)

Description:

~  

Here's the structure of the testing framework for the reviewbotext directory. It doesn't account for the evolutions, static, or templates sub directories right now but I'm not sure which of those will need testing (some of its CSS and javascript and stuff). My current plan is to extend the ReviewBotExtensionTestsExtension class with a subclass for every file in the directory to better separate the testing suites.

  ~

Fixed comment length (changed to 70 characters)

   
~  

I'm going through nose, notion, and review board documentation right now, and some of the examples are a little different, so please let me know if I've made any design errors or things that could be structured better, thanks.

  ~

Added .nosids and .test_notes to .gitignore (Note: I'm hoping I did it properly. I may have only added the file name and not a path so please let me know if I messed up).

   
~  

Also, I'm having some issues importing the ReviewBot class so any info on what I'm doing wrong would be greatly appreciated.

  ~

Moved all subclasses from the previous runtests.py file into their own files.

  +
  +

Added more empty function definitions in the new files that match the tests I have planned for that file.

  +
  +

Also, I'm assuming I'm going to have flake8 errors again. I'm not exactly sure how to execute "flake8 --ignore E121,E125,E129,E241 --max-line-length 79 PATH" locally as it gives me an error in my terminal. If anyone knows what I'm doing wrong please let me know.

Commit:

-d285412985d8701c3f2ad545694cb4a4cfe34168
+b0dbd60c49331ac52fc86cb90350258dd3f5baae

Diff:

Revision 2 (+76361)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

david
  1. One suggestion as far as approach: you seem to be going for a breadth-first implementation, creating empty test case classes and methods for every possible thing. Planning is good, but for the code it's probably going to be better if you focus on a depth-first approach, getting a single test case implemented and working, and then building on that.

  2. 
      
Loading...