Integration test fix ppa #1296
Conversation
| "simplestreams-dev-ubuntu-trunk-{}.list".format(release) | ||
| ) | ||
| host = "launchpad" if release == "jammy" else "launchpadcontent" | ||
| host = "launchpadcontent" if release == "jammy" else "launchpad" |
There was a problem hiding this comment.
LGTM, but this is going to fail again Ubuntu KK. What about this diff?
- host = "launchpad" if release == "jammy" else "launchpadcontent"
assert (
- f"://ppa.{host}.net/simplestreams-dev/trunk/ubuntu"
+ f"://ppa.launchpad.net/simplestreams-dev/trunk/ubuntu"
+ in ppa_path_contents
+ or f"://ppa.launchpadcontent.net/simplestreams-dev/trunk/ubuntu"
in ppa_path_contents
)
There was a problem hiding this comment.
Or we can keep the existing logic but check if release >= "jammy" to make the fix more future proof.
There was a problem hiding this comment.
... What about this diff?
- host = "launchpad" if release == "jammy" else "launchpadcontent" assert ( - f"://ppa.{host}.net/simplestreams-dev/trunk/ubuntu" + f"://ppa.launchpad.net/simplestreams-dev/trunk/ubuntu" + in ppa_path_contents + or f"://ppa.launchpadcontent.net/simplestreams-dev/trunk/ubuntu" in ppa_path_contents )
I'm going with this. Since release is just a string of the release name, I think this works better than your next suggestion.
There was a problem hiding this comment.
Still it would work until ZZ as release names are alphabetically ordered, but maybe the or is just simpler.
There was a problem hiding this comment.
@holmanb Sorry to originally prescribe more strict validation here. I was thinking in KK release time-frame we would grow an ImageSpecification.gt/lt dunder set of methods to make this future-proof
At the moment go with what you have as your last suggestion. The asserting is correct anyway although you don't need the f"" string now.
holmanb
force-pushed
the
holmanb/integration-test-fix-ppa-try-2
branch
from
b8e59fd to
e0519ad
Compare
paride
left a comment
paride
left a comment
There was a problem hiding this comment.
+1, but we'll need a committer approval.
blackboxsw
left a comment
blackboxsw
left a comment
There was a problem hiding this comment.
validated lxd_container runs locally on both focal and jammy.
CLOUD_INIT_OS_IMAGE=jammy::ubuntu::jammy .tox/integration-tests/bin/pytest tests/integration_tests/modules/test_apt.py::TestApt::test_ppa_source
permissively allow either launchpadcontent or launchpad
permissively allow either launchpadcontent or launchpad
permissively allow either launchpadcontent or launchpad
3 participants
Per https://github.com/canonical/cloud-init/pull/1287/files/a677b2486f8a945d35c0f9a5d295063aa9b69209#r812973166, the url conditional is backwards.