Merge ppa-dev-tools:fix-lp-test into ppa-dev-tools:main

Proposed by Bryce Harrington
Status: Merged
Merge reported by: Bryce Harrington
Merged at revision: 572c3cfb8e7a15f15d44ae0b7eb28e050156e904
Proposed branch: ppa-dev-tools:fix-lp-test
Merge into: ppa-dev-tools:main
Diff against target: 174 lines (+56/-49)
2 files modified
ppa/lp.py (+1/-1)
tests/test_lp.py (+55/-48)
Reviewer Review Type Date Requested Status
Athos Ribeiro (community) Approve
Canonical Server Reporter Pending
Review via email: mp+429716@code.staging.launchpad.net

Description of the change

This finishes the implementation of test cases for the Launchpad object.

There may be some cleaner ways to implement some of the logic; there's a lot to MagicMock that I haven't mastered yet.

To post a comment you must log in.
Revision history for this message
Athos Ribeiro (athos-ribeiro) wrote :

Nice!

Thanks Bryce, LGTM.

While working on running the test suite, I realized xdg is only used to fetch the home configuration directory. I wonder if there is a better way to find the lp credentials without depending on xdg. How does LP writes that file?

review: Approve
Revision history for this message
Bryce Harrington (bryce) wrote :

"While working on running the test suite, I realized xdg is only used to fetch the home configuration directory. I wonder if there is a better way to find the lp credentials without depending on xdg. How does LP writes that file?"

It's a good question. The lp.py codebase is lifted from Bileto, which was placing the credentials right in ~/.launchpad.credentials. I added xdg to at least move this into ~/.config/ or whatever the user has specified as their config directory. However, if we just omit the credentials_file= argument then it'll pick an appropriate default. Robie mentioned this has the nice side effect of letting it share credentials with other LP-using tools.

Since that's a separate fix from these, I'll include that change in a followup branch.

Revision history for this message
Athos Ribeiro (athos-ribeiro) wrote :

Tanks, Bryce!

review: Approve

There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.

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: