Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

launchpad: catch import error if httplib2 is missing and move tests into tests/integration/ #233

Merged
merged 3 commits into from Oct 26, 2023

Conversation

bdrung
Copy link
Collaborator

@bdrung bdrung commented Oct 25, 2023

The launchpadlib Python modules uses and depends on the httplib2 module. If launchpadlib is not installed, httplib2 might not be installed.

Move importing httplib2 to the guarded launchpadlib import which sets Launchpad to None in an import failure case. Then the user will get an error message presented on usage.

The Launchpad crash DB tests should not be inside the apport module to avoid installing them on all systems. Therefore move it to the integration tests.

Previously the Launchpad tests could be run by calling:

python3 apport/crashdb_impl/launchpad.py

Now they can be run by calling:

python3 -m unittest tests/integration/test_crashdb_launchpad.py

The tests log in to https://qastaging.launchpad.net and the user need to have bug control permissions on the Ubuntu coreutils package. They need around seven minutes to execute. Do not run them by default because a broken QA staging Launchpad instance will break them and there is no special Launchpad user for CI runs.

To enable these tests in the CI, the tests should be modified to use python3-vcr or launchpadlib.testing.launchpad.FakeLaunchpad. This is a task for the future.

The launchpadlib Python modules uses and depends on the httplib2 module.
If launchpadlib is not installed, httplib2 might not be installed.

Move importing httplib2 to the guarded launchpadlib import which sets
`Launchpad` to `None` in an import failure case. Then the user will get
an error message presented on usage.

Signed-off-by: Benjamin Drung <benjamin.drung@canonical.com>
@codecov
Copy link

codecov bot commented Oct 25, 2023

Codecov Report

Merging #233 (5c6317a) into main (3124d91) will increase coverage by 0.66%.
The diff coverage is 13.76%.

@@            Coverage Diff             @@
##             main     #233      +/-   ##
==========================================
+ Coverage   82.99%   83.66%   +0.66%     
==========================================
  Files          92       93       +1     
  Lines       18744    18758      +14     
==========================================
+ Hits        15557    15694     +137     
+ Misses       3187     3064     -123     
Files Coverage Δ
apport/crashdb_impl/launchpad.py 11.35% <100.00%> (+11.35%) ⬆️
tests/integration/test_crashdb_launchpad.py 13.60% <13.60%> (ø)

@bdrung
Copy link
Collaborator Author

bdrung commented Oct 25, 2023

The complaining codecov/patch should be ignored.

Copy link
Contributor

@schopin-pro schopin-pro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice find, I was surprised to find tests in that impl file :)

For future work, I think we want both cassette tests and tests that tap into the staging LP site, with the latter marked as not-blocking. That way we can actually detect whenever the LP folks break our stuff and gently nudge them before it's too late ;)

tests/integration/test_crashdb_launchpad.py Show resolved Hide resolved
@schopin-pro
Copy link
Contributor

As usual, feel free to merge if you want to disregard my nitpick.

The Launchpad tests log in to https://qastaging.launchpad.net and the
user need to have bug control permissions on the Ubuntu coreutils
package. Do not run them by default because a broken QA staging
Launchpad instance will break them and there is no special Launchpad
user for CI runs.

To enable these tests in the CI, the tests should be modified to use
python3-vcr or `launchpadlib.testing.launchpad.FakeLaunchpad`.

Signed-off-by: Benjamin Drung <benjamin.drung@canonical.com>
The Launchpad crash DB tests should not be inside the `apport` module to
avoid installing them on all systems. Therefore move it to the
integration tests.

Previously the Launchpad tests could be run by calling:

```
TEST_LAUNCHPAD=1 python3 apport/crashdb_impl/launchpad.py
```

Now they can be run by calling:

```
TEST_LAUNCHPAD=1 python3 -m unittest tests/integration/test_crashdb_launchpad.py
```

The tests log in to https://qastaging.launchpad.net and the user need to
have bug control permissions on the Ubuntu coreutils package. They need
around seven minutes to execute.

Signed-off-by: Benjamin Drung <benjamin.drung@canonical.com>
@schopin-pro schopin-pro merged commit e14c1e6 into canonical:main Oct 26, 2023
24 of 25 checks passed
@bdrung bdrung deleted the move-launchpad-tests branch October 26, 2023 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants