Merge ~racb/ubuntu-sponsoring:git-ubuntu-applied-branches into ubuntu-sponsoring:main

[Robie Basak]

This branch does two things:

1) It displays git-ubuntu applied branches in the sponsorship report, was my intent when I started the branch. To make this easier to develop I added unit tests. To make the unit tests work I had to rearrange the code so that it's all in a module that can be found by pytest, using entry_points= instead of scripts= in setup.py, and arrange for the tests to run on package build.

2) There's a separate bug that stopped the sponsorship report from working over the weekend due to an escaping issue. I fixed this in the final commit as I discovered it during testing.

If you want you can cherry-pick just the latter issue. I could have submitted it as a separate MP, but that's painful because I have moved the script itself, and it seems likely this will result in conflicts to resolve if cherry-picking or trying to merge the two changes separately. If we merge them together then that will save that work.

[Robie Basak (racb)]

You might want to review this with --find-renames

[Benjamin Drung (bdrung)]

I cherry-picked the top commit to fix the generation failure. I'll review the remaining commit tomorrow.

From a quick glance: "Make sponsors-page a proper entry point" could be the first commit to avoid creating a symlink to then remove that later again.

[Robie Basak (racb)]

> From a quick glance: "Make sponsors-page a proper entry point" could be the first commit to avoid creating a symlink to then remove that later again.

Agreed. But I didn't know or expect that I'd need to fix that before I tried to get the tests to run as part of the build, and they did run correctly from the development tree. I didn't think it was worth trying to rebase everything through the rename to reorder it, given that the commits are correct as-is and the state of the source tree at the end is also the ideal result.

I was about to rejiggle this for you but I see that you've also moved to pyproject.toml now so it needs redoing differently. That's fine but is it OK if I leave it to you then please to refactor as you wish?

[Robie Basak (racb)]

> The README.md needs an update.

I suggest just:

-./sponsors-page
+python3 ubuntu_sponsoring/sponsors_page.py

Unmerged commits

5fed1c2... by Robie Basak on 2024-07-22

Fix escaping in string substitutions

re.sub() is defined to accept and interpret escapes in the replacement
string. This is not what we want. Currently r'\e' is present in the bug
title of a sponsorship request, and this causes the report generation to
fail with an exception since that is not a valid escape.

Instead, use plain str.replace(), which does not do such substitutions.
There is no need to use regular expressions to find simple string
substrings, and this avoids the issue since str.replace() is not defined
to do any interpretation of the replacement string.

c4f179d... by Robie Basak on 2024-07-22

Run tests at build time

ffc2745... by Robie Basak on 2024-07-22

Make sponsors-page a proper entry point

I'd like to be able to test functions inside sponsors-page, and that's
difficult with it being a standalone script at the top level.

If we use the entry point functionality of setuptools, then it will
manage the top-level script for us automatically, and then all the
useful code can be with a function inside the ubuntu_sponsoring module,
making all the code we ship testable.

Then we can drop the sponsors_page.py symlink, too.

7838b0f... by Robie Basak on 2024-06-10

Display MPs against git-ubuntu applied branches

I noticed that the sponsoring page wasn't showing an MP which
(incorrectly) targetted applied/ubuntu/noble-devel. But we should
display these regardless.

694a0f6... by Robie Basak on 2024-06-10

Add tests for get_git_ubuntu_target_series()

3b99311... by Robie Basak on 2024-06-10

Add sponsors_page.py symlink

This allows us to import the main script for testing purposes.

diff --git a/debian/control b/debian/control
index 1e0ff47..ff5c721 100644
--- a/debian/control
+++ b/debian/control
@@ -10,6 +10,7 @@ Build-Depends: debhelper-compat (= 12),
                python3-cachetools,
                python3-launchpadlib,
                python3-lxml,
+               python3-pytest,
                python3-setuptools
 Testsuite: autopkgtest-pkg-python
 Standards-Version: 4.6.2
diff --git a/setup.py b/setup.py
index 94a58da..8c5fef7 100644
--- a/setup.py
+++ b/setup.py
@@ -9,7 +9,9 @@ setup(
     author="Benjamin Drung <[e-mail]>",
     requires=["bs4", "cachetools", "launchpadlib", "lxml"],
     packages=["ubuntu_sponsoring"],
-    scripts=["sponsors-page"],
+    entry_points={'console_scripts': [
+        'sponsors-page=ubuntu_sponsoring.sponsors_page:main',
+    ]},
     data_files=[
         (
             "share/ubuntu-sponsoring",
@@ -24,4 +26,5 @@ setup(
             ],
         )
     ],
+    tests_require=['pytest'],
 )
diff --git a/sponsors-page b/ubuntu_sponsoring/sponsors_page.py
similarity index 93%
rename from sponsors-page
rename to ubuntu_sponsoring/sponsors_page.py
index 482104b..a940de0 100755
--- a/sponsors-page
+++ b/ubuntu_sponsoring/sponsors_page.py
@@ -15,7 +15,6 @@ import logging
 import operator
 import os
 import pathlib
-import re
 import shutil
 import sys
 from typing import Collection, Iterable, MutableMapping, Optional, Union
@@ -57,7 +56,7 @@ LP_UBUNTU_DISTRIBUTION = "https://api.launchpad.net/devel/ubuntu"
 LOG_FORMAT = "%(asctime)-15s %(levelname)s: %(message)s"
 __script_name__ = os.path.basename(sys.argv[0]) if __name__ == "__main__" else __name__
 
-if __file__.startswith("/usr/bin"):
+if __file__.startswith("/usr/lib"):
     DATA_DIR = pathlib.Path("/usr/share/ubuntu-sponsoring")
 else:
     DATA_DIR = pathlib.Path(sys.argv[0]).parent
@@ -88,9 +87,9 @@ def htmlify_name(person, package, devs, distribution):
 def classify_name(prefix, name):
     name = name.upper()
     if prefix == "status":
-        name = re.sub(" ", "", name)
+        name = name.replace(" ", "")
     else:
-        name = re.sub(" ", "_", name)
+        name = name.replace(" ", "_")
     return prefix + name
 
 
@@ -574,9 +573,15 @@ def get_branches(lp, distribution, team):
 
 
 def get_git_ubuntu_target_series(ubuntu, target_git_path):
-    if target_git_path.startswith("refs/heads/ubuntu/"):
-        effective_target_git_path = target_git_path
-    elif target_git_path == "refs/heads/debian/sid":
+    if target_git_path.startswith("refs/heads/applied/"):
+        unapplied_target_git_path = (
+            "refs/heads/" + target_git_path.removeprefix("refs/heads/applied/")
+        )
+    else:
+        unapplied_target_git_path = target_git_path
+    if unapplied_target_git_path.startswith("refs/heads/ubuntu/"):
+        effective_target_git_path = unapplied_target_git_path
+    elif unapplied_target_git_path == "refs/heads/debian/sid":
         # We use "debian/sid" for package merge MPs. LP: 1976112
         effective_target_git_path = "refs/heads/ubuntu/devel"
     else:
@@ -772,7 +777,7 @@ def generate_page(
     template = DATA_DIR / "template.html"
     html_report = template.read_text(encoding="utf-8")
     for pattern, substitution in subst.items():
-        html_report = re.sub("@" + pattern + "@", str(substitution), html_report)
+        html_report = html_report.replace("@" + pattern + "@", str(substitution))
 
     if os.path.exists(filename):
         os.remove(filename)
diff --git a/ubuntu_sponsoring/sponsors_page_test.py b/ubuntu_sponsoring/sponsors_page_test.py
new file mode 100644
index 0000000..48b2e69
--- /dev/null
+++ b/ubuntu_sponsoring/sponsors_page_test.py
@@ -0,0 +1,57 @@
+import unittest.mock
+
+import pytest
+
+import ubuntu_sponsoring.sponsors_page as target
+
+
[e-mail](
+    ["current_series", "target_git_path", "expected_result"],
+    [
+        ("noble", "refs/heads/ubuntu/noble", "noble"),
+        ("noble", "refs/heads/ubuntu/noble-devel", "noble"),
+        ("noble", "refs/heads/ubuntu/jammy-devel", "jammy"),
+        ("noble", "refs/heads/ubuntu/noble-proposed", "noble"),
+        ("noble", "refs/heads/ubuntu/noble-updates", "noble"),
+        ("noble", "refs/heads/ubuntu/devel", "noble"),
+        ("noble", "refs/heads/debian/sid", "noble"),
+        ("noble", "refs/heads/applied/ubuntu/devel", "noble"),
+        ("noble", "refs/heads/applied/ubuntu/noble-devel", "noble"),
+        ("noble", "refs/heads/applied/ubuntu/jammy-devel", "jammy"),
+    ],
+)
+def test_get_git_ubuntu_target_series(
+    current_series, target_git_path, expected_result
+):
+    # 'name' is special so we cannot use __init__ **kwarg since that
+    # behaviour would be different.
+    current_series_mock = unittest.mock.Mock()
+    current_series_mock.configure_mock(name=current_series)
+    assert (
+        target.get_git_ubuntu_target_series(
+            unittest.mock.Mock(current_series=current_series_mock),
+            target_git_path,
+        )
+        == expected_result
+    )
+
+
[e-mail](
+    ["current_series", "target_git_path", "expected_exception"],
+    [
+        ("noble", "refs/heads/debian/bullseye", ValueError),
+        ("noble", "refs/heads/other", ValueError),
+    ],
+)
+def test_get_git_ubuntu_target_series_exceptions(
+    current_series, target_git_path, expected_exception
+):
+    # 'name' is special so we cannot use __init__ **kwarg since that
+    # behaviour would be different.
+    current_series_mock = unittest.mock.Mock()
+    current_series_mock.configure_mock(name=current_series)
+    with pytest.raises(expected_exception):
+        target.get_git_ubuntu_target_series(
+            unittest.mock.Mock(current_series=current_series_mock),
+            target_git_path,
+        )