Summary

Add test to verify that static files exist

Review Request #8603 — Created Jan. 7, 2017 and submitted Jan. 10, 2017, 3:45 p.m.

Information

Owner

erijo

Repository

Review Board

Branch

master

Bugs

Depends On

Commit

56e62a1...

Reviewers

Groups

reviewboard

People

Description

The test verifies that all files referenced in staticbundles.py exists.

Remove reference to:
* rb/js/admin/models/supportContractModel.js: reference, but no file, added in
6a857664
* rb/js/views/fileAttachmentCommentBlockView.js: reference, but no file, added
in 7b3a4b6d
* rb/js/diffviewer.js: file removed in 05ba1ae3
* rb/js/reviews.js: file removed in d7cdfa6d

Testing Done


Creating test database for alias 'default'...
#888 Testing that all static javascript files exist ... ok
#889 Testing that all static stylesheet files exist ... ok

----------------------------------------------------------------------
Ran 2 tests in 1.373s

OK

Issues

Description	From	Last Updated
Add a file level docstring indicating what this file is. The first import should be from __future__ import unicode_literals.	brennie	Jan. 10, 2017, 11:08 a.m.
Missing a class docstring.	brennie	Jan. 10, 2017, 11:08 a.m.
Missing method docstring. Private method names in testcases should be prefixed with an underscore.	brennie	Jan. 10, 2017, 11:09 a.m.
Missing method docstring.	brennie	Jan. 10, 2017, 11:09 a.m.
Missing method docstring.	brennie	Jan. 10, 2017, 11:09 a.m.
There should be a blank line between these; imports go into 3 groups: stdlib, "third party" (including djblets), and "this …	david	Jan. 10, 2017, 11:31 a.m.
Blank line between these two.	david	Jan. 10, 2017, 11:31 a.m.
Nicer formatting might be: static_path = os.path.join('reviewboard', 'static', path) if not os.path.exists(static_path): missing.add(path)	david	Jan. 10, 2017, 11:31 a.m.
Our testcase docstrings are all of the format "Testing ...". They also shouldn't contain the trailing period, since the test …	david	Jan. 10, 2017, 11:31 a.m.
Same here.	david	Jan. 10, 2017, 11:31 a.m.
Blank line between these two, please.	david	Jan. 10, 2017, 11:52 a.m.

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/tests.py



Tool: Pyflakes
Processed Files:
    reviewboard/tests.py

Description:

		The test verifies that all files referenced in staticbundles.py exists.

~		This change could have been part of https://reviews.reviewboard.org/r/8604/, but is posted as a separate review as the test currently fails and I'm not sure what to do with the other missing files.
	~	Remove reference to:
	+	* rb/js/admin/models/supportContractModel.js: reference, but no file, added in
	+	6a857664
	+	* rb/js/views/fileAttachmentCommentBlockView.js: reference, but no file, added
	+	in 7b3a4b6d
	+	* rb/js/diffviewer.js: file removed in 05ba1ae3
	+	* rb/js/reviews.js: file removed in d7cdfa6d

Testing Done:



		`#884 test_javascript_bundles (reviewboard.tests.StaticBundlesTests) ... FAIL`
		#885 test_stylesheets_bundles (reviewboard.tests.StaticBundlesTests) ... ok

		======================================================================
		FAIL: test_javascript_bundles (reviewboard.tests.StaticBundlesTests)
		----------------------------------------------------------------------
		Traceback (most recent call last):
		File "/home/erik/source/reviewboard/reviewboard/tests.py", line 22, in test_javascript_bundles
		self.check_static_bundles(PIPELINE_JAVASCRIPT)
		File "/home/erik/source/reviewboard/reviewboard/tests.py", line 19, in check_static_bundles
		self.assertSetEqual(missing, set())
		AssertionError: Items in the first set but not the second:
-		u'rb/js/admin/models/supportContractModel.js'
-		u'rb/js/diffviewer.js'
		u'rb/js/diffviewer/models/diffCommentBlockModel.js'
-		u'rb/js/views/fileAttachmentCommentBlockView.js'
-		u'rb/js/reviews.js'

		----------------------------------------------------------------------
~		Ran 2 tests in 0.435s
	~	Ran 2 tests in 0.439s
-
-		FAILED (failures=1)

Commit:

fa84548b671ca3701b49c292bf63903f875da14c

00fab5202e384aa486206fdf0520762dd17b6062

Diff:

Revision 2 (+25 -4)

Show changes

	reviewboard/staticbundles.py
	reviewboard/tests.py

Tool: Pyflakes
Processed Files:
    reviewboard/tests.py
    reviewboard/staticbundles.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/tests.py
    reviewboard/staticbundles.py

reviewboard/tests.py (Diff revision 2)

The issue has been resolved. Show all issues

Add a file level docstring indicating what this file is.
The first import should be from __future__ import unicode_literals.

reviewboard/tests.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Missing a class docstring.
```

reviewboard/tests.py (Diff revision 2)

The issue has been resolved. Show all issues

Missing method docstring.

Private method names in testcases should be prefixed with an underscore.

reviewboard/tests.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Missing method docstring.
```
reviewboard/tests.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Missing method docstring.
```

Change Summary:

Fixing review comments and minor refactoring of the test.

Testing Done:



~		`#884 test_javascript_bundles (reviewboard.tests.StaticBundlesTests) ... FAIL`
~		#885 test_stylesheets_bundles (reviewboard.tests.StaticBundlesTests) ... ok
	~	`Creating test database for alias 'default'...`
	~	#888 Tests that all static javascript files exists. ... ok
	+	#889 Tests that all static stylesheet files exists. ... ok

-		======================================================================
-		FAIL: test_javascript_bundles (reviewboard.tests.StaticBundlesTests)
		----------------------------------------------------------------------
~		Traceback (most recent call last):
	~	Ran 2 tests in 1.373s
-		File "/home/erik/source/reviewboard/reviewboard/tests.py", line 22, in test_javascript_bundles
-		self.check_static_bundles(PIPELINE_JAVASCRIPT)
-		File "/home/erik/source/reviewboard/reviewboard/tests.py", line 19, in check_static_bundles
-		self.assertSetEqual(missing, set())
-		AssertionError: Items in the first set but not the second:
-		u'rb/js/diffviewer/models/diffCommentBlockModel.js'

~		----------------------------------------------------------------------
	~	OK
-		Ran 2 tests in 0.439s

Commit:

00fab5202e384aa486206fdf0520762dd17b6062

52209a564ef359f693c0ca3d06563abae1d21096

Diff:

Revision 3 (+47 -4)

Show changes

	reviewboard/staticbundles.py
	reviewboard/tests.py

Tool: Pyflakes
Processed Files:
    reviewboard/tests.py
    reviewboard/staticbundles.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/tests.py
    reviewboard/staticbundles.py

reviewboard/tests.py (Diff revision 3)

The issue has been resolved. Show all issues

There should be a blank line between these; imports go into 3 groups: stdlib, "third party" (including djblets), and "this module".

reviewboard/tests.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Blank line between these two.
```

reviewboard/tests.py (Diff revision 3)

The issue has been resolved. Show all issues

Nicer formatting might be:

static_path = os.path.join('reviewboard', 'static', path)

if not os.path.exists(static_path):
    missing.add(path)

reviewboard/tests.py (Diff revision 3)

The issue has been resolved. Show all issues

Our testcase docstrings are all of the format "Testing ...". They also shouldn't contain the trailing period, since the test runner adds an ellipsis.
Proper grammar should also be "all static javascript files exist"

reviewboard/tests.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Same here.
```

Testing Done:



		`Creating test database for alias 'default'...`
~		#888 Tests that all static javascript files exists. ... ok
~		#889 Tests that all static stylesheet files exists. ... ok
	~	#888 Testing that all static javascript files exist ... ok
	~	#889 Testing that all static stylesheet files exist ... ok

		----------------------------------------------------------------------
		Ran 2 tests in 1.373s

		OK

Commit:

52209a564ef359f693c0ca3d06563abae1d21096

920074890a879d79e7908215df4c45b526675739

Diff:

Revision 4 (+49 -4)

Show changes

	reviewboard/staticbundles.py
	reviewboard/tests.py

Tool: Pyflakes
Processed Files:
    reviewboard/tests.py
    reviewboard/staticbundles.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/tests.py
    reviewboard/staticbundles.py

```
Just one trivial thing left.
```
reviewboard/tests.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Blank line between these two, please.
```

Change Summary:

blank_line++

Commit:

920074890a879d79e7908215df4c45b526675739

56e62a1d8bbbb0393e0c5081a247b0ef81e63054

Diff:

Revision 5 (+50 -4)

Show changes

	reviewboard/staticbundles.py
	reviewboard/tests.py

Tool: Pyflakes
Processed Files:
    reviewboard/tests.py
    reviewboard/staticbundles.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/tests.py
    reviewboard/staticbundles.py

Ship it!

```
Ship It!
```

Ship it!

I'm going to land this with one tiny last-minute change that I noticed. Thanks so much for this!

Status:: Completed
Change Summary:: Pushed to release-3.0.x (eece297)