Skip to content

Refactored test framework#32

Open
erz9engel wants to merge 30 commits intomainfrom
DE-1684/test_framework_refactoring
Open

Refactored test framework#32
erz9engel wants to merge 30 commits intomainfrom
DE-1684/test_framework_refactoring

Conversation

@erz9engel
Copy link
Collaborator

@erz9engel erz9engel commented Mar 3, 2026

Existing tests in the repo were basically e2e/integration tests and they require real API key. Some of the tests even require access to some paid features.
Updated test framework to include 2 parts: unit tests and integration tests.
Added some specific unit tests.
Coverage is currently 80%.
Updated Makefile commands to run unit tests and different tests sets (including or excluding coverage check)

@erz9engel erz9engel requested a review from skupriienko March 3, 2026 12:15
@erz9engel erz9engel self-assigned this Mar 3, 2026
@erz9engel erz9engel changed the title Upsaframework refactoring Refactored test framework Mar 3, 2026
result = BaseEndpoint.build_url(
url, domain="example.com", method="get", domain_name="example.com"
)
assert "example.com" in result

Check failure

Code scanning / CodeQL

Incomplete URL substring sanitization High test

The string
example.com
may be at an arbitrary position in the sanitized URL.

Copilot Autofix

AI 1 day ago

In general, to avoid incomplete URL substring sanitization, the code should parse the URL and check the host (and possibly path) using a URL parser rather than using "example.com" in url or url.endswith("example.com"). In tests, this means asserting on parsed components instead of generic substring presence.

Here, the best fix is to change the test so that it asserts that the hostname of the built URL equals the expected domain (example.com), rather than that "example.com" appears somewhere in the full URL string. We can do this by importing urlparse from Python’s standard library urllib.parse and using it to extract the hostname from result.

Concretely:

  • Add an import at the top of tests/unit/test_client.py:
    • from urllib.parse import urlparse
  • Update test_build_url_domains_with_domain:
    • Replace assert "example.com" in result with parsing the URL and asserting urlparse(result).hostname == "example.com" (or at least that the hostname equals the expected value). This keeps the functional intent (verifying that the domain is correctly embedded) but in a more precise and secure way.
  • No other tests need to change, and this does not alter production behavior—only the test assertion becomes stricter and more correct.
Suggested changeset 1
tests/unit/test_client.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py
--- a/tests/unit/test_client.py
+++ b/tests/unit/test_client.py
@@ -5,6 +5,7 @@
 
 import pytest
 import requests
+from urllib.parse import urlparse
 
 from mailgun.client import BaseEndpoint
 from mailgun.client import Client
@@ -51,7 +52,8 @@
         result = BaseEndpoint.build_url(
             url, domain="example.com", method="get", domain_name="example.com"
         )
-        assert "example.com" in result
+        parsed = urlparse(result)
+        assert parsed.hostname == "example.com"
 
     def test_build_url_domainlist(self) -> None:
         url = {"base": "https://api.mailgun.net/v4/", "keys": ["domainlist"]}
EOF
@@ -5,6 +5,7 @@

import pytest
import requests
from urllib.parse import urlparse

from mailgun.client import BaseEndpoint
from mailgun.client import Client
@@ -51,7 +52,8 @@
result = BaseEndpoint.build_url(
url, domain="example.com", method="get", domain_name="example.com"
)
assert "example.com" in result
parsed = urlparse(result)
assert parsed.hostname == "example.com"

def test_build_url_domainlist(self) -> None:
url = {"base": "https://api.mailgun.net/v4/", "keys": ["domainlist"]}
Copilot is powered by AI and may make mistakes. Always verify output.
def test_builds_url_with_keys(self) -> None:
url = {"base": "https://api.mailgun.net/v3/", "keys": ["events"]}
result = handle_default(url, "example.com", "get")
assert "example.com" in result

Check failure

Code scanning / CodeQL

Incomplete URL substring sanitization High test

The string
example.com
may be at an arbitrary position in the sanitized URL.

Copilot Autofix

AI 1 day ago

In general, to fix incomplete URL substring checks, you should either (a) assert the full expected URL string, or (b) parse the URL and assert specific structural components (scheme, host, path, etc.) instead of using "...example.com..." in url. In tests, this means expressing stricter expectations so implementations must construct URLs in a well-defined, safe way.

For this concrete case, the best fix that does not change existing functionality is:

  • For TestHandleDefault.test_builds_url_with_keys, replace the two substring assertions:
assert "example.com" in result
assert "events" in result

with a single assertion on the exact URL that should be produced, consistent with the preceding test test_builds_url_with_domain. That test shows that with base "https://api.mailgun.net/v3/", domain "example.com", and keys ["messages"], the expected URL is "https://api.mailgun.net/v3/example.com/messages". By analogy, when keys is ["events"], the expected URL is "https://api.mailgun.net/v3/example.com/events".

No new imports or helper methods are needed; we only tighten the assertion to compare against the full URL string. This removes the substring-based URL “sanitization” pattern that CodeQL is warning about while preserving the intended functional check.

Suggested changeset 1
tests/unit/test_handlers.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/tests/unit/test_handlers.py b/tests/unit/test_handlers.py
--- a/tests/unit/test_handlers.py
+++ b/tests/unit/test_handlers.py
@@ -40,10 +40,10 @@
     def test_builds_url_with_keys(self) -> None:
         url = {"base": "https://api.mailgun.net/v3/", "keys": ["events"]}
         result = handle_default(url, "example.com", "get")
-        assert "example.com" in result
-        assert "events" in result
+        assert result == "https://api.mailgun.net/v3/example.com/events"
 
 
+
 class TestHandleDomainlist:
     """Tests for handle_domainlist."""
 
EOF
@@ -40,10 +40,10 @@
def test_builds_url_with_keys(self) -> None:
url = {"base": "https://api.mailgun.net/v3/", "keys": ["events"]}
result = handle_default(url, "example.com", "get")
assert "example.com" in result
assert "events" in result
assert result == "https://api.mailgun.net/v3/example.com/events"



class TestHandleDomainlist:
"""Tests for handle_domainlist."""

Copilot is powered by AI and may make mistakes. Always verify output.
def test_with_domain_and_keys(self) -> None:
url = {"base": "https://api.mailgun.net/v4/domains/", "keys": ["webhooks"]}
result = handle_domains(url, "example.com", "get")
assert "example.com" in result

Check failure

Code scanning / CodeQL

Incomplete URL substring sanitization High test

The string
example.com
may be at an arbitrary position in the sanitized URL.

Copilot Autofix

AI 1 day ago

In general, the problem is that the test uses a substring match on the full URL ("example.com" in result) instead of asserting the expected URL structure. To avoid incomplete substring checks, tests should compare against a fully specified expected URL, or parse the URL and assert on its components (scheme, netloc, path). This makes the intention explicit and avoids patterns that resemble unsafe domain checks.

For this codebase, the safest, least invasive fix is to change the test to assert the full expected URL for handle_domains when called with base https://api.mailgun.net/v4/domains/, domain example.com, method "get", and key "webhooks". The other assertion assert "webhooks" in result can either remain or also be made more precise; however, the primary CodeQL complaint is about "example.com" in result, so replacing that with an exact equality is sufficient and does not change the behavior the test is trying to enforce (it still passes only if the domain is included correctly). No new imports or helpers are necessary: we update just the assertion line (and optionally its neighbors) in tests/unit/test_handlers.py.

Concretely:

  • In TestHandleDomains.test_with_domain_and_keys, replace the substring assertion on "example.com" with an equality check that the entire URL is "https://api.mailgun.net/v4/domains/example.com/webhooks".
  • Leave the rest of the tests unchanged.
Suggested changeset 1
tests/unit/test_handlers.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/tests/unit/test_handlers.py b/tests/unit/test_handlers.py
--- a/tests/unit/test_handlers.py
+++ b/tests/unit/test_handlers.py
@@ -59,7 +59,7 @@
     def test_with_domain_and_keys(self) -> None:
         url = {"base": "https://api.mailgun.net/v4/domains/", "keys": ["webhooks"]}
         result = handle_domains(url, "example.com", "get")
-        assert "example.com" in result
+        assert result == "https://api.mailgun.net/v4/domains/example.com/webhooks"
         assert "webhooks" in result
 
     def test_requires_domain_when_keys_present(self) -> None:
EOF
@@ -59,7 +59,7 @@
def test_with_domain_and_keys(self) -> None:
url = {"base": "https://api.mailgun.net/v4/domains/", "keys": ["webhooks"]}
result = handle_domains(url, "example.com", "get")
assert "example.com" in result
assert result == "https://api.mailgun.net/v4/domains/example.com/webhooks"
assert "webhooks" in result

def test_requires_domain_when_keys_present(self) -> None:
Copilot is powered by AI and may make mistakes. Always verify output.
result = handle_domains(
url, None, "get", domain_name="my-domain.com"
)
assert "my-domain.com" in result

Check failure

Code scanning / CodeQL

Incomplete URL substring sanitization High test

The string
my-domain.com
may be at an arbitrary position in the sanitized URL.

Copilot Autofix

AI 1 day ago

In general, the problem here is that the test only verifies that "my-domain.com" appears somewhere in the result string, which mirrors the insecure “substring containment” pattern discouraged by the guidelines. The fix is to make the test assert the exact URL returned, or at least the exact path component where the domain must appear, instead of a loose substring check.

The best fix without changing functionality is to change the assertion in TestHandleDomains.test_with_domain_name_kwarg_get so it expects a specific URL formed from the known base and the domain_name argument. The test constructs url = {"base": "https://api.mailgun.net/v4/domains/", "keys": []} and then calls handle_domains(url, None, "get", domain_name="my-domain.com"). From the neighboring tests and naming, the most consistent expected behavior is that handle_domains returns a URL like https://api.mailgun.net/v4/domains/my-domain.com. We should assert equality with that full string instead of checking for substring containment. No new imports or helper methods are needed; we only modify the relevant assertion line in tests/unit/test_handlers.py.

Concretely:

  • In tests/unit/test_handlers.py, in TestHandleDomains.test_with_domain_name_kwarg_get, replace assert "my-domain.com" in result with assert result == "https://api.mailgun.net/v4/domains/my-domain.com" (preserving indentation). This keeps the test focused and precise and removes the problematic substring check.
Suggested changeset 1
tests/unit/test_handlers.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/tests/unit/test_handlers.py b/tests/unit/test_handlers.py
--- a/tests/unit/test_handlers.py
+++ b/tests/unit/test_handlers.py
@@ -77,7 +77,7 @@
         result = handle_domains(
             url, None, "get", domain_name="my-domain.com"
         )
-        assert "my-domain.com" in result
+        assert result == "https://api.mailgun.net/v4/domains/my-domain.com"
 
     def test_verify_requires_true(self) -> None:
         url = {"base": "https://api.mailgun.net/v4/domains/", "keys": []}
EOF
@@ -77,7 +77,7 @@
result = handle_domains(
url, None, "get", domain_name="my-domain.com"
)
assert "my-domain.com" in result
assert result == "https://api.mailgun.net/v4/domains/my-domain.com"

def test_verify_requires_true(self) -> None:
url = {"base": "https://api.mailgun.net/v4/domains/", "keys": []}
Copilot is powered by AI and may make mistakes. Always verify output.
def test_builds_tags_url_with_domain(self) -> None:
url = {"base": "https://api.mailgun.net/v3/", "keys": ["tags"]}
result = handle_tags(url, "example.com", None)
assert "example.com" in result

Check failure

Code scanning / CodeQL

Incomplete URL substring sanitization High test

The string
example.com
may be at an arbitrary position in the sanitized URL.

Copilot Autofix

AI 1 day ago

In general, to avoid incomplete URL substring checks, you should either (a) compare the full expected URL string, or (b) parse the URL and assert on specific components such as scheme, host, path, and query, instead of checking that a domain substring appears anywhere.

Here, the best fix that does not change functional behavior is to make the test assert the exact URL that handle_tags is expected to produce, rather than asserting that "example.com" appears somewhere in the result. The other tests in this file already use stronger assertions such as assert result == "https://api.mailgun.net/v3/example.com/messages" and assert result == "https://api.mailgun.net/v3/ips", so aligning this test with that pattern both removes the substring check and makes the test stricter and clearer.

Concretely, in tests/unit/test_handlers.py, within TestHandleTags.test_builds_tags_url_with_domain, replace:

assert "example.com" in result
assert "tags" in result

with a single assertion that the entire URL is exactly what is expected, e.g.:

assert result == "https://api.mailgun.net/v3/example.com/tags"

This assumes handle_tags constructs URLs consistently with the other handlers: base + domain + key. No additional imports or helper methods are needed.

Suggested changeset 1
tests/unit/test_handlers.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/tests/unit/test_handlers.py b/tests/unit/test_handlers.py
--- a/tests/unit/test_handlers.py
+++ b/tests/unit/test_handlers.py
@@ -137,8 +137,7 @@
     def test_builds_tags_url_with_domain(self) -> None:
         url = {"base": "https://api.mailgun.net/v3/", "keys": ["tags"]}
         result = handle_tags(url, "example.com", None)
-        assert "example.com" in result
-        assert "tags" in result
+        assert result == "https://api.mailgun.net/v3/example.com/tags"
 
     def test_with_tag_name_kwarg(self) -> None:
         url = {"base": "https://api.mailgun.net/v3/", "keys": ["tags"]}
EOF
@@ -137,8 +137,7 @@
def test_builds_tags_url_with_domain(self) -> None:
url = {"base": "https://api.mailgun.net/v3/", "keys": ["tags"]}
result = handle_tags(url, "example.com", None)
assert "example.com" in result
assert "tags" in result
assert result == "https://api.mailgun.net/v3/example.com/tags"

def test_with_tag_name_kwarg(self) -> None:
url = {"base": "https://api.mailgun.net/v3/", "keys": ["tags"]}
Copilot is powered by AI and may make mistakes. Always verify output.
def test_with_domain(self) -> None:
url = {"base": "https://api.mailgun.net/v3/", "keys": ["bounces"]}
result = handle_bounces(url, "example.com", None)
assert "example.com" in result

Check failure

Code scanning / CodeQL

Incomplete URL substring sanitization High test

The string
example.com
may be at an arbitrary position in the sanitized URL.

Copilot Autofix

AI 1 day ago

In general, to avoid incomplete URL substring sanitization, you should parse or deterministically construct URLs and validate specific components (scheme, host, path), rather than searching for substrings anywhere within the URL string. Tests should mirror this by asserting full, structured expectations instead of using broad substring membership checks.

For this specific test, we can strengthen the assertion to check that handle_bounces returns exactly the URL we expect for the given inputs. Other tests in the same file (e.g., test_builds_url_with_domain in TestHandleDefault and test_base_without_trailing_slash in TestHandleIps) already assert the full URL equality, so this change aligns with existing style and doesn’t alter intended functionality. Assuming handle_bounces constructs URLs as "{base}{domain}/{key}" with base = "https://api.mailgun.net/v3/" and key = "bounces", the expected URL string is "https://api.mailgun.net/v3/example.com/bounces".

Concretely:

  • In tests/unit/test_handlers.py, within TestHandleBounces.test_with_domain, replace:
    • assert "example.com" in result
    • assert "bounces" in result
  • With a single equality assertion:
    • assert result == "https://api.mailgun.net/v3/example.com/bounces"

No new imports or helper methods are needed.

Suggested changeset 1
tests/unit/test_handlers.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/tests/unit/test_handlers.py b/tests/unit/test_handlers.py
--- a/tests/unit/test_handlers.py
+++ b/tests/unit/test_handlers.py
@@ -152,8 +152,7 @@
     def test_with_domain(self) -> None:
         url = {"base": "https://api.mailgun.net/v3/", "keys": ["bounces"]}
         result = handle_bounces(url, "example.com", None)
-        assert "example.com" in result
-        assert "bounces" in result
+        assert result == "https://api.mailgun.net/v3/example.com/bounces"
 
     def test_with_bounce_address(self) -> None:
         url = {"base": "https://api.mailgun.net/v3/", "keys": ["bounces"]}
EOF
@@ -152,8 +152,7 @@
def test_with_domain(self) -> None:
url = {"base": "https://api.mailgun.net/v3/", "keys": ["bounces"]}
result = handle_bounces(url, "example.com", None)
assert "example.com" in result
assert "bounces" in result
assert result == "https://api.mailgun.net/v3/example.com/bounces"

def test_with_bounce_address(self) -> None:
url = {"base": "https://api.mailgun.net/v3/", "keys": ["bounces"]}
Copilot is powered by AI and may make mistakes. Always verify output.
def test_with_domain(self) -> None:
url = {"base": "https://api.mailgun.net/v3/", "keys": ["whitelists"]}
result = handle_whitelists(url, "example.com", None)
assert "example.com" in result

Check failure

Code scanning / CodeQL

Incomplete URL substring sanitization High test

The string
example.com
may be at an arbitrary position in the sanitized URL.

Copilot Autofix

AI about 2 hours ago

In general, to fix incomplete URL substring sanitization, you should parse and inspect structured components of the URL (scheme, host, path, query) instead of searching for substrings. In tests, a more precise and safer pattern is to assert the exact URL or at least well-defined URL components, not just that a particular string appears somewhere in the result.

For this specific test file, we can avoid substring checks that mention the domain and instead assert either the full URL returned by the handler or, if that is not known, at least that the URL begins with the expected base and path sequence. We must not change how the handlers behave, just how the tests verify them. The minimal change that removes the flagged pattern while preserving intent is:

  • In TestHandleWhitelists.test_with_domain, replace:
    • assert "example.com" in result
    • assert "whitelists" in result
  • With a single assertion of the exact URL we expect based on the existing patterns in the file: "https://api.mailgun.net/v3/example.com/whitelists".

Other tests in this file already assert full URLs (for example, the TestHandleDefault and TestHandleAddressValidate tests). Aligning TestHandleWhitelists with these tests makes behavior clearer and more robust. No new imports or helper methods are needed; we only adjust the assertion text in the shown region.

Suggested changeset 1
tests/unit/test_handlers.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/tests/unit/test_handlers.py b/tests/unit/test_handlers.py
--- a/tests/unit/test_handlers.py
+++ b/tests/unit/test_handlers.py
@@ -191,8 +191,7 @@
     def test_with_domain(self) -> None:
         url = {"base": "https://api.mailgun.net/v3/", "keys": ["whitelists"]}
         result = handle_whitelists(url, "example.com", None)
-        assert "example.com" in result
-        assert "whitelists" in result
+        assert result == "https://api.mailgun.net/v3/example.com/whitelists"
 
 
 class TestHandleAddressValidate:
EOF
@@ -191,8 +191,7 @@
def test_with_domain(self) -> None:
url = {"base": "https://api.mailgun.net/v3/", "keys": ["whitelists"]}
result = handle_whitelists(url, "example.com", None)
assert "example.com" in result
assert "whitelists" in result
assert result == "https://api.mailgun.net/v3/example.com/whitelists"


class TestHandleAddressValidate:
Copilot is powered by AI and may make mistakes. Always verify output.
@erz9engel erz9engel force-pushed the DE-1684/test_framework_refactoring branch 2 times, most recently from f043857 to 29ac88b Compare March 3, 2026 15:01
@erz9engel erz9engel force-pushed the DE-1684/test_framework_refactoring branch from 29ac88b to fe56a9a Compare March 3, 2026 17:00
@erz9engel erz9engel force-pushed the DE-1684/test_framework_refactoring branch from a50e086 to 4406bc0 Compare March 4, 2026 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant