Closed Bug 673935 Opened 13 years ago Closed 13 years ago

[pushes] create tests for diff view

Categories

(Webtools Graveyard :: Elmo, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Pike, Assigned: Pike)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
Priority: -- → P2
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 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-
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 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+
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
Attachment #567593 - Flags: review?(stas)
Product: Webtools → Webtools Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: