Landscape Client

Merge lp://staging/~tribaal/landscape-client/fix-1429888-wrong-credentials-stacktrace into lp://staging/~landscape/landscape-client/trunk

  1. fix-1429888-wrong-credentials-stacktrace
  2. Merge into trunk
Proposed by Chris Glass
Status: Merged
Approved by: Chris Glass
Approved revision: 821
Merged at revision: 814
Proposed branch: lp://staging/~tribaal/landscape-client/fix-1429888-wrong-credentials-stacktrace
Merge into: lp://staging/~landscape/landscape-client/trunk
Diff against target: 144 lines (+72/-17)
2 files modified
landscape/configuration.py (+11/-6)
landscape/tests/test_configuration.py (+61/-11)
To merge this branch: bzr merge lp://staging/~tribaal/landscape-client/fix-1429888-wrong-credentials-stacktrace
Reviewer Review Type Date Requested Status
Björn Tillenius (community) Approve
Adam Collard (community) Approve
Review via email: mp+252703@code.staging.launchpad.net

Commit message

This branch fixes the related bug, where inappropriate account/password combinations made client registration badly.

Description of the change

This branch fixes the related bug, where inappropriate account/password combinations made client registration badly.

Test with:
- Spin up an LXC
- Install landscape-client
- symlink /usr/lib/python2.7/dist-packages/landscape to a checkout of this branch
- run landscape-config, enter a wrong registration password
- re-run landscape-config, enter a correct registration password.
- The client registers.

To post a comment you must log in.
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Once this lands, please remove the skip tag from the robot test in landscape/trunk (server):

=== modified file 'robot/system/test/client/upgrade.txt'
--- robot/system/test/client/upgrade.txt 2015-03-10 18:45:51 +0000
+++ robot/system/test/client/upgrade.txt 2015-03-12 16:07:01 +0000
@@ -5,8 +5,7 @@
 Suite Setup Client Upgrade Suite Setup
 Suite Teardown Client Upgrade Suite Teardown
 Test Setup Default Setup
-# XXX remove skip tag after #1429888 is fixed
-Force Tags client-upgrade skip
+Force Tags client-upgrade

 *** Variables ***

Revision history for this message
Adam Collard (adam-collard) wrote :

Question inline

review: Needs Information
Revision history for this message
Chris Glass (tribaal) :
Revision history for this message
Chris Glass (tribaal) :
lp://staging/~tribaal/landscape-client/fix-1429888-wrong-credentials-stacktrace updated
817. By Chris Glass

Made tests more explicit (hopefully)

Revision history for this message
Adam Collard (adam-collard) wrote :

Spellings

Revision history for this message
Adam Collard (adam-collard) wrote :

Two inline comments

Revision history for this message
Benji York (benji) :
Revision history for this message
Chris Glass (tribaal) :
lp://staging/~tribaal/landscape-client/fix-1429888-wrong-credentials-stacktrace updated
818. By Chris Glass

PEP-257

819. By Chris Glass

Fixed comments.

Revision history for this message
Chris Glass (tribaal) wrote :

All comments should be addressed.

Revision history for this message
Adam Collard (adam-collard) wrote :

Looks good, +1.

FWIW I tested by making packages and then installing those :)

review: Approve
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
lp://staging/~tribaal/landscape-client/fix-1429888-wrong-credentials-stacktrace updated
820. By Chris Glass

Added test. No idea why this one fails.

821. By Chris Glass

The test passes now. Thanks for the insight Bjorn!

Revision history for this message
Chris Glass (tribaal) wrote :

All comments should be addressed.

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

Thanks for the added test. +1.

review: Approve
Revision history for this message
Chris Glass (tribaal) wrote :

For the record I removed the skip tag in the server's robot test now that this landed, see: https://bazaar.launchpad.net/~landscape/landscape/trunk/revision/8616

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: