#194 increasing unittests, for hubs/edit
Merged by skrzepto. Opened by skrzepto.
skrzepto/fedora-hubs unittest  into  unittest

Download 194.patch

kindof figured out how to test redirection

Whats being tested in //edit
- Update hub with no data
- Update hub with valid data
- Update hub with invalid data

Misc
- pep8 the file

with this we are at 60% coverage

Nice! Looks good to me. :thumbsup:

Does it work without this line here ^?

One comment, the rest looks good to me as well :)

Unfortunately no :( I could never get the the @app.before_request def check_auth(): too get called to set g.auth. Without this line it keeps complaining that g.auth.loggedin is not a variable or something like that. This was my kind of work around :/

I can take a look again at http://flask.pocoo.org/docs/0.11/api/#flask.Flask.before_request

to get that working instead of the way im loading it

So maybe we should adjust the code to check if there is an auth attribute to flask.g

Maybe something like https://pagure.io/pagure/blob/master/f/pagure/init.py#_156 ?

i like that a lot :) let me see if I can get that implemented

rebased

pep8 files and added a new function to see if flask.g.auth is set

This seems wrong, I think authenticated() should cover all cases, no?

These pep8 changes seem odd to me but that might just be me :)

rebased

Pingou i made the authenticated function do all the checking.

also are we strict on pep8 80 chars? i really don't like that limit 100 would be nice

also are we strict on pep8 80 chars?

I kinda like it :)

I only ask about the pep8 char limit since this kind of looks funny split. But i'll follow the convention of 80 chars

What about simplifying it to :
return hasattr(flask.g, 'auth') and flask.g.auth is not None and flask.g.auth.logged_in

But on these occasion I don't think we should apply pep8, at all :)

what if the session has the login stored? I could shorten up this function but I feel like we should still call check_auth() if this returns false hasattr(flask.g, 'auth') and flask.g.auth is not None and flask.g.auth.logged_in

It should be called automatically, otherwise I'd consider if it's not a bug in flask or in our usage of it

Okay :) there's a few more spaces i'd like to exempt the 80 char limit ill note them as a comment.

Considering that this is an URL, temporary and that the part behind the hash sign is not a comment per say, I'd just ignore pep8 here :)

rebased

But here we could follow the 80 chars rule and split at the and to make a condition per line :)

rebased

LGTM. :ship: it

Pull-Request has been merged by skrzepto

Metadata