Code review comment for lp:~tribaal/landscape-client/fix-1429888-wrong-credentials-stacktrace

Revision history for this message
Björn Tillenius (bjornt) wrote :

I think we need to try to test this better. The style of testing you use is the same style that didn't caught this error, so I don't have much confidence in those tests. IMO, they don't test that things actually work, they test that the code is written in a certain way.

I think you should try to add a test in RegisterRealFunctionTest suite, so that we can see that register() actually returns a sane error. I would trust that more than I trust the comment you added.

Note that you will have to change FakeRemoteBroker, so that you can tell it to make register() to fail. But it shouldn't be too hard. Ping me if you have any problems.

review: Needs Fixing

« Back to merge proposal