#86 Adding some unit test to trac importer
Merged by cverna. Opened by cverna.
cverna/pagure-importer lets_add_some_test  into  master

Download 86.patch

Couple of unit test added, as we keep adding new feature this should make it easier to test in the future

Is this purpose of this method to make method_name default to runTest?

I recommend making these cases be separate test methods. It'll make the code easier to follow, and they can also pass/fail independently.

I would also recommend splitting all of these test conditions into separate test methods, for the same reasons I stated earlier. Basically, each call to get_ticket_status() should ideally be its own test method. You can use a setUp() to do the first two lines in the method so you don't have to repeat that code.

Ah sorry, I meant the first line. But honestly, you could just put a {} in the call to get_ticket_status() and then you don't need to define trac_ticket. Then you don't even need setUp() to share common code between these because there won't be any common code ☺

If you use a test runner to run the tests, you don't need this block.

This looks acceptable to me, but I strongly recommend breaking those tests up into lots of small test methods so they are easier to read, and so they can fail or pass independently (which is handy when tracking down regressions).

haha just copied from a tutorial on how to use the unittest module :)

Thanks for the review :), I ll make the changes.

rebased

1 new commit added

  • Adding test for custom fields

rebased

I recommend squashing your commits into a single atomic commit.

Maybe put a space between $ and python.

LGTM, my only remaining suggestion is to squash your commits so you have a cleaner git log.

rebased

Thanks for the review ;)

Pull-Request has been merged by cverna

Metadata