Closed
Bug 673935
Opened 13 years ago
Closed 13 years ago
[pushes] create tests for diff view
Categories
(Webtools Graveyard :: Elmo, defect, P2)
Webtools Graveyard
Elmo
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: Pike, Assigned: Pike)
References
Details
Attachments
(1 file, 1 obsolete file)
6.62 KB,
patch
|
peterbe
:
review+
|
Details | Diff | Splinter Review |
The diff view should have tests, see bug 667900 for example. Tests should cover: diffs that pass diffs that trigger parser-known and -not-known file types diffs that trigger errors, bad encoding would be a start diffs that trigger errors for bad revisions diffs that trigger errors for good revisions that are in separate related repos (should be made successful later on) diffs that trigger errors for good revisions in unrelated repos (should stay an error) Probably best to set up the test repos programmatically in a test temp dir, and then hit the test cases. Those can use the fixrepos command to update the database for the test repos.
Assignee | ||
Updated•13 years ago
|
Priority: -- → P2
Assignee | ||
Comment 1•13 years ago
|
||
The test here is in the wrong position, it should be in pushes. I've put it in life, but that's just me being a chicken and knowing that this still depends on the move of the diff view into pushes, and then I'll have merge hell. Also, the test doesn't cover all the bases I've suggested when filing this bug. But either way, this is in a good state to get some feedback.
Assignee: nobody → l10n
Status: NEW → ASSIGNED
Attachment #558850 -
Flags: feedback?(stas)
Attachment #558850 -
Flags: feedback?(peterbe)
Comment 2•13 years ago
|
||
Comment on attachment 558850 [details] [diff] [review] wip, first attempt to set up a stage and run some tests on diff Review of attachment 558850 [details] [diff] [review]: ----------------------------------------------------------------- I don't really understand the hgcommands stuff because I'm not familiar with it but in terms of syntax it looks great! The non-intuitive @raises() decorator is the only reason to r-. ::: apps/life/tests/test_repos.py @@ +71,5 @@ > + > + > +def tearDownModule(): > + """Delete the fake repositories""" > + shutil.rmtree(settings.REPOSITORY_BASE) What if this directory doesn't exist (when tests fail)? Will shutil.rmtree() raise IOErrors? @@ +136,5 @@ > + 'to': to_[:12]}) > + eq_(response.status_code, 200) > + > + # FIXME: right now, we can't diff between repos, so this > + @raises(RepoError) Since we're actually using `unittest` and not nose I think it's better to use: `self.assertRaises(RepoError, callable, args, ..)` because the method decorator doesn't indicate where the error is raised.
Attachment #558850 -
Flags: feedback?(peterbe) → feedback-
Assignee | ||
Comment 3•13 years ago
|
||
With review comments addressed.
Attachment #558850 -
Attachment is obsolete: true
Attachment #558850 -
Flags: feedback?(stas)
Attachment #567593 -
Flags: review?(stas)
Attachment #567593 -
Flags: review?(peterbe)
Comment 4•13 years ago
|
||
Comment on attachment 567593 [details] [diff] [review] address review comments Review of attachment 567593 [details] [diff] [review]: ----------------------------------------------------------------- My concerns were address. I'd love to see this landed on develop asap.
Attachment #567593 -
Flags: review?(peterbe) → review+
Assignee | ||
Comment 5•13 years ago
|
||
https://github.com/mozilla/elmo/commit/2679039b96c0c34c4f8fb24d9ef8b53f7b75004f, with bustage fix https://github.com/mozilla/elmo/commit/f181b55b188cf8656a890bf39ea4bc5855b85556. FIXED.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•12 years ago
|
Attachment #567593 -
Flags: review?(stas)
Updated•4 years ago
|
Product: Webtools → Webtools Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•