Review Request 32: Tests for html template rendering

[Tim Flink]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-tflink.rhcloud.com/r/32/#review38
-----------------------------------------------------------



blockerbugs/controllers/main.py
<http://reviewboard-tflink.rhcloud.com/r/32/#comment55>

    Was this supposed to make it into the main controller? This method is not OK to make public - while it would fail due to permissions in production it would effectively clear the database when anyone visited <base_url>/update_db



blockerbugs/controllers/main.py
<http://reviewboard-tflink.rhcloud.com/r/32/#comment61>

    merge error? I don't think this is an intentional change



blockerbugs/models/release.py
<http://reviewboard-tflink.rhcloud.com/r/32/#comment62>

    good catch



blockerbugs/util/bug_sync.py
<http://reviewboard-tflink.rhcloud.com/r/32/#comment60>

    I'm not a fan of changes like this, in general. In this particular case, it works well enough but in many cases it really decreases legibility.



blockerbugs/util/bz_interface.py
<http://reviewboard-tflink.rhcloud.com/r/32/#comment59>

    I think this is a merge issue or a diff display issue - this reverts a bugfix that martin pushed a while ago



testing/test_bugchange.py
<http://reviewboard-tflink.rhcloud.com/r/32/#comment56>

    Is this just an issue with how rb is displaying diffs, a merge issue or is there a reason you're removing a test here?



testing/test_controllers.py
<http://reviewboard-tflink.rhcloud.com/r/32/#comment58>

    I think that a more descriptive method name would be good here - something like test_emptydb_index_returns_200code



testing/test_controllers.py
<http://reviewboard-tflink.rhcloud.com/r/32/#comment57>

    These should ideally be split up into separate tests so that we know exactly what failed if anything does fail - it feels a bit verbose but cramming all these assertions into the same methods is a bad habit to get into


Looks decent so far - a couple of issues that I suspect are either a merge problem or your branch needs to be updated.

- Tim Flink


On July 2, 2013, 4:22 p.m., Ilgiz Islamgulov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard-tflink.rhcloud.com/r/32/
> -----------------------------------------------------------
> 
> (Updated July 2, 2013, 4:22 p.m.)
> 
> 
> Review request for blockerbugs.
> 
> 
> Repository: blockerbugs
> 
> 
> Description
> -------
> 
> Simple tests for rendering html templates.
> 
> 
> Diffs
> -----
> 
>   testing/test_controllers.py PRE-CREATION 
>   testing/test_bugchange.py 7dfeb80fec3354aa998d02ed18bc94c1bb2e5530 
>   blockerbugs/util/bz_interface.py 5ac5699cf1fcb0f37631c79deeb44c4f8119e51f 
>   blockerbugs/util/bug_sync.py 49cce49740cd6f5b1f430f58c8d1b522e1f0b7e3 
>   blockerbugs/templates/base_nav.html 021ddb3126b4c0f30a231d8d9b32df09c669280e 
>   blockerbugs/models/release.py 2f74a69cec0b7769d4e6c21b7a4a84105c5d8a64 
>   blockerbugs/controllers/main.py 6c3b1de09819fa37fccf6652dbadb43da3a72c63 
> 
> Diff: http://reviewboard-tflink.rhcloud.com/r/32/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ilgiz Islamgulov
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.fedoraproject.org/pipermail/qa-devel/attachments/20130703/ed3e47f4/attachment-0001.html>