Code review comment for lp:~pfalcon/linaro-license-protection/crowd-auth

Revision history for this message
Paul Sokolovsky (pfalcon) wrote :

> Not something I'm really against, but I would be more interested in testing only
the status code I get, not the kind of "string" I will receive (to me that looks
already more than an unit test). I would test something like that:

Well, if looking in more detail, what I did is factored out that string construction out of OpenIdAuth class. The was also test_openid_auth which exactly did that string test. Consequently, I moved that string test where its new routine belongs. I didn't add anything, I don't feel comfortable removing anything either - my idea was to preserve original semantics and tests which cover it.

> I personally try to avoid concatenating strings in that way.

Again, routine wasn't written by me ;-)

« Back to merge proposal