Summary

WIP Added testing skeleton for the reviewbotext directory

Review Request #11237 — Created Oct. 21, 2020 and updated Nov. 9, 2020, 12:43 p.m.

Information

Owner

jace

Repository

ReviewBot

Branch

master

Bugs

Depends On

Commit

b0dbd60...

Reviewers

Groups

reviewbot

People

ceciliawei, jblazusi

Description

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.

Testing Done

Issues

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. …	ceciliawei	Oct. 27, 2020, 4:51 p.m.
This file is auto-generated and shouldn't be part of your commit. Let's remove it from this and instead add .noseids …	david	Oct. 27, 2020, 4:51 p.m.
Rather than call this runtests.py and have this shebang (which indicates that it should be run as a script), let's …	david	Oct. 21, 2020, 10:13 p.m.
The top-level module name is reviewbotext (ReviewBot and extension are just directories in the source). So these should be: from …	david	Oct. 21, 2020, 10:13 p.m.
E302 expected 2 blank lines, found 1	reviewbot	Oct. 21, 2020, 9:35 p.m.
E999 SyntaxError: invalid syntax	reviewbot	Oct. 21, 2020, 9:35 p.m.
E302 expected 2 blank lines, found 1	reviewbot	Oct. 21, 2020, 9:35 p.m.
E302 expected 2 blank lines, found 1	reviewbot	Oct. 21, 2020, 9:35 p.m.
E302 expected 2 blank lines, found 1	reviewbot	Oct. 21, 2020, 9:35 p.m.
E302 expected 2 blank lines, found 1	reviewbot	Oct. 21, 2020, 9:35 p.m.
E302 expected 2 blank lines, found 1	reviewbot	Oct. 21, 2020, 9:35 p.m.
E302 expected 2 blank lines, found 1	reviewbot	Oct. 21, 2020, 9:35 p.m.
E302 expected 2 blank lines, found 1	reviewbot	Oct. 21, 2020, 9:35 p.m.
E302 expected 2 blank lines, found 0	reviewbot	Oct. 21, 2020, 9:35 p.m.
E302 expected 2 blank lines, found 1	reviewbot	Oct. 21, 2020, 9:35 p.m.
E302 expected 2 blank lines, found 1	reviewbot	Nov. 3, 2020, 8:22 p.m.
E101 indentation contains mixed spaces and tabs	reviewbot	Nov. 3, 2020, 8:22 p.m.
W191 indentation contains tabs	reviewbot	Nov. 3, 2020, 8:22 p.m.
E999 IndentationError: unexpected indent	reviewbot	Nov. 3, 2020, 8:22 p.m.
E113 unexpected indentation	reviewbot	Nov. 3, 2020, 8:22 p.m.
E101 indentation contains mixed spaces and tabs	reviewbot	Nov. 3, 2020, 8:22 p.m.
E115 expected an indented block (comment)	reviewbot	Nov. 3, 2020, 8:22 p.m.
E112 expected an indented block	reviewbot	Nov. 3, 2020, 8:22 p.m.
E101 indentation contains mixed spaces and tabs	reviewbot	Nov. 3, 2020, 8:22 p.m.
E303 too many blank lines (2)	reviewbot	Nov. 3, 2020, 8:22 p.m.
W191 indentation contains tabs	reviewbot	Nov. 3, 2020, 8:22 p.m.
W191 indentation contains tabs	reviewbot	Nov. 3, 2020, 8:22 p.m.
W191 indentation contains tabs	reviewbot	Nov. 3, 2020, 8:22 p.m.
E303 too many blank lines (2)	reviewbot	Nov. 3, 2020, 8:22 p.m.
W191 indentation contains tabs	reviewbot	Nov. 3, 2020, 8:22 p.m.
W191 indentation contains tabs	reviewbot	Nov. 3, 2020, 8:22 p.m.
E303 too many blank lines (2)	reviewbot	Nov. 3, 2020, 8:22 p.m.
W191 indentation contains tabs	reviewbot	Nov. 3, 2020, 8:22 p.m.
W191 indentation contains tabs	reviewbot	Nov. 3, 2020, 8:22 p.m.
E303 too many blank lines (2)	reviewbot	Nov. 3, 2020, 8:22 p.m.
W191 indentation contains tabs	reviewbot	Nov. 3, 2020, 8:22 p.m.
W191 indentation contains tabs	reviewbot	Nov. 3, 2020, 8:22 p.m.
E303 too many blank lines (2)	reviewbot	Nov. 3, 2020, 8:22 p.m.
W191 indentation contains tabs	reviewbot	Nov. 3, 2020, 8:22 p.m.
W191 indentation contains tabs	reviewbot	Nov. 3, 2020, 8:22 p.m.
W191 indentation contains tabs	reviewbot	Nov. 3, 2020, 8:22 p.m.
E265 block comment should start with '# '	reviewbot	Nov. 3, 2020, 8:22 p.m.
E302 expected 2 blank lines, found 1	reviewbot	Nov. 3, 2020, 8:22 p.m.
E999 SyntaxError: invalid syntax	reviewbot	Nov. 3, 2020, 8:22 p.m.
E101 indentation contains mixed spaces and tabs	reviewbot	Nov. 3, 2020, 8:22 p.m.

flake8 failed.

JSHint passed.

flake8

extension/tests/runtests.py (Diff revision 1)
Show all issues
```
E302 expected 2 blank lines, found 1
```
extension/tests/runtests.py (Diff revision 1)
Show all issues
```
E999 SyntaxError: invalid syntax
```
extension/tests/runtests.py (Diff revision 1)
Show all issues
```
E302 expected 2 blank lines, found 1
```
extension/tests/runtests.py (Diff revision 1)
Show all issues
```
E302 expected 2 blank lines, found 1
```
extension/tests/runtests.py (Diff revision 1)
Show all issues
```
E302 expected 2 blank lines, found 1
```
extension/tests/runtests.py (Diff revision 1)
Show all issues
```
E302 expected 2 blank lines, found 1
```
extension/tests/runtests.py (Diff revision 1)
Show all issues
```
E302 expected 2 blank lines, found 1
```
extension/tests/runtests.py (Diff revision 1)
Show all issues
```
E302 expected 2 blank lines, found 1
```
extension/tests/runtests.py (Diff revision 1)
Show all issues
```
E302 expected 2 blank lines, found 1
```
extension/tests/runtests.py (Diff revision 1)
Show all issues
```
E302 expected 2 blank lines, found 0
```
extension/tests/runtests.py (Diff revision 1)
Show all issues
```
E302 expected 2 blank lines, found 1
```

Summary:

-	added testing skeleton for the reviewbotext directory
+	WIP Added testing skeleton for the reviewbotext directory

Show all issues


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.

Fix it!

extension/tests/.noseids (Diff revision 1)

Show all issues

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.

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

Show all issues

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.

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

Show all issues

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

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)

david Oct. 22, 2020, 11:52 a.m.

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.

david Oct. 22, 2020, 11:52 a.m.
```
Err, TestCase.setUp, not UnitTest.
```

jblazusi Oct. 22, 2020, 11:55 a.m.

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

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

	.gitignore
	extension/os
	extension/sys
	extension/tests/.noseids
	extension/tests/runtests.py
	extension/tests/test_admin.py
	extension/tests/test_adminurls.py
	extension/tests/test_extension.py
	8 more

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

```
Warning: Showing 30 of 64 failures.
```
extension/tests/test_admin.py (Diff revision 2)
Show all issues
```
E302 expected 2 blank lines, found 1
```
extension/tests/test_admin.py (Diff revision 2)
Show all issues
```
E101 indentation contains mixed spaces and tabs
```
extension/tests/test_admin.py (Diff revision 2)
Show all issues
```
W191 indentation contains tabs
```
extension/tests/test_admin.py (Diff revision 2)
Show all issues
```
E999 IndentationError: unexpected indent
```
extension/tests/test_admin.py (Diff revision 2)
Show all issues
```
E113 unexpected indentation
```
extension/tests/test_admin.py (Diff revision 2)
Show all issues
```
E101 indentation contains mixed spaces and tabs
```
extension/tests/test_admin.py (Diff revision 2)
Show all issues
```
E115 expected an indented block (comment)
```
extension/tests/test_admin.py (Diff revision 2)
Show all issues
```
E112 expected an indented block
```
extension/tests/test_admin.py (Diff revision 2)
Show all issues
```
E101 indentation contains mixed spaces and tabs
```
extension/tests/test_admin.py (Diff revision 2)
Show all issues
```
E303 too many blank lines (2)
```
extension/tests/test_admin.py (Diff revision 2)
Show all issues
```
W191 indentation contains tabs
```
extension/tests/test_admin.py (Diff revision 2)
Show all issues
```
W191 indentation contains tabs
```
extension/tests/test_admin.py (Diff revision 2)
Show all issues
```
W191 indentation contains tabs
```
extension/tests/test_admin.py (Diff revision 2)
Show all issues
```
E303 too many blank lines (2)
```
extension/tests/test_admin.py (Diff revision 2)
Show all issues
```
W191 indentation contains tabs
```
extension/tests/test_admin.py (Diff revision 2)
Show all issues
```
W191 indentation contains tabs
```
extension/tests/test_admin.py (Diff revision 2)
Show all issues
```
E303 too many blank lines (2)
```
extension/tests/test_admin.py (Diff revision 2)
Show all issues
```
W191 indentation contains tabs
```
extension/tests/test_admin.py (Diff revision 2)
Show all issues
```
W191 indentation contains tabs
```
extension/tests/test_admin.py (Diff revision 2)
Show all issues
```
E303 too many blank lines (2)
```
extension/tests/test_admin.py (Diff revision 2)
Show all issues
```
W191 indentation contains tabs
```
extension/tests/test_admin.py (Diff revision 2)
Show all issues
```
W191 indentation contains tabs
```
extension/tests/test_admin.py (Diff revision 2)
Show all issues
```
E303 too many blank lines (2)
```
extension/tests/test_admin.py (Diff revision 2)
Show all issues
```
W191 indentation contains tabs
```
extension/tests/test_admin.py (Diff revision 2)
Show all issues
```
W191 indentation contains tabs
```
extension/tests/test_admin.py (Diff revision 2)
Show all issues
```
W191 indentation contains tabs
```
extension/tests/test_admin.py (Diff revision 2)
Show all issues
```
E265 block comment should start with '# '
```
extension/tests/test_adminurls.py (Diff revision 2)
Show all issues
```
E302 expected 2 blank lines, found 1
```
extension/tests/test_adminurls.py (Diff revision 2)
Show all issues
```
E999 SyntaxError: invalid syntax
```
extension/tests/test_adminurls.py (Diff revision 2)
Show all issues
```
E101 indentation contains mixed spaces and tabs
```

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.