From 254e76f9e778c0929063a4a0bd22df417304be0e Mon Sep 17 00:00:00 2001 From: Martin Kourim Date: Mon, 3 Aug 2026 17:26:43 +0200 Subject: [PATCH 01/13] docs(blockers): correct fixed_in docstring The docstring claimed fixed_in is ignored on unknown projects, but the code compares it against the cardano-node version. Existing issue definitions rely on this behavior, so document it instead. --- cardano_node_tests/utils/blockers.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cardano_node_tests/utils/blockers.py b/cardano_node_tests/utils/blockers.py index fb823d691..5004b578b 100644 --- a/cardano_node_tests/utils/blockers.py +++ b/cardano_node_tests/utils/blockers.py @@ -22,7 +22,9 @@ class GH: Attributes: issue: A GitHub issue number. repo: A repository where the issue belongs to. Default: `IntersectMBO/cardano-node`. - fixed_in: A version of the project where the issue is fixed. Ignored on unknown projects. + fixed_in: A version of the project where the issue is fixed. On projects other than + cardano-node, cardano-cli and cardano-db-sync, it is the cardano-node version + into which the fix was integrated. message: A message to be added to blocking outcome. """ From 81aa07fdd2c97cef50141fd1d1c230b66fbd34be Mon Sep 17 00:00:00 2001 From: Martin Kourim Date: Mon, 3 Aug 2026 17:27:05 +0200 Subject: [PATCH 02/13] fix(blockers): raise error on nonexistent issue A typo in an issue number resulted in state "unknown", which was treated as an open issue, so the test was silently xfailed forever. Raise ValueError instead so the wrong issue reference surfaces immediately. Transient failures ("get_state_failure", missing GitHub instance) still conservatively assume the issue is blocked. --- cardano_node_tests/utils/blockers.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/cardano_node_tests/utils/blockers.py b/cardano_node_tests/utils/blockers.py index 5004b578b..70c3eb8b1 100644 --- a/cardano_node_tests/utils/blockers.py +++ b/cardano_node_tests/utils/blockers.py @@ -60,8 +60,16 @@ def _issue_blocked_in_version(self, product_version: version.Version) -> bool: ) return True + state = self.gh_issue.get_state() + + # Fail early when the issue doesn't exist, e.g. because of a typo in the issue number. + # Otherwise the test would be silently xfailed forever. + if state == "unknown": + msg = f"Issue '{self.repo}#{self.issue}' doesn't exist" + raise ValueError(msg) + # The issue is blocked if it is was not closed yet - if not self.gh_issue.is_closed(): + if state != "closed": return True # The issue is blocked if it was fixed or integrated into a product version that is greater From 3865d271fb3588c8739b5964893d5d99671f0481 Mon Sep 17 00:00:00 2001 From: Martin Kourim Date: Mon, 3 Aug 2026 17:27:18 +0200 Subject: [PATCH 03/13] fix(blockers): reject empty issues in finish_test Calling finish_test with an empty iterable silently xfailed the test with an empty reason, masking a bug in the caller. Raise ValueError instead. --- cardano_node_tests/utils/blockers.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cardano_node_tests/utils/blockers.py b/cardano_node_tests/utils/blockers.py index 70c3eb8b1..f9b4095d9 100644 --- a/cardano_node_tests/utils/blockers.py +++ b/cardano_node_tests/utils/blockers.py @@ -127,6 +127,9 @@ def _get_outcome(issue: GH) -> tuple[bool, str, str]: return blocked, reason, log_message outcomes = [_get_outcome(i) for i in issues] + if not outcomes: + msg = "No issues were provided" + raise ValueError(msg) should_fail = False for blocked, __, log_message in outcomes: From 033d075af4597613ce823621adb74cdcb20548f2 Mon Sep 17 00:00:00 2001 From: Martin Kourim Date: Mon, 3 Aug 2026 17:28:10 +0200 Subject: [PATCH 04/13] fix(blockers): parse fixed_in at issue definition An invalid fixed_in version string raised InvalidVersion only when the blocked check ran inside a test, looking like a test failure. Parse it eagerly in __init__ so the error points at the issue definition. Also avoids reparsing on every check. --- cardano_node_tests/utils/blockers.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/cardano_node_tests/utils/blockers.py b/cardano_node_tests/utils/blockers.py index f9b4095d9..879f20fa7 100644 --- a/cardano_node_tests/utils/blockers.py +++ b/cardano_node_tests/utils/blockers.py @@ -38,6 +38,8 @@ def __init__( self.issue = issue self.repo = repo self.fixed_in = fixed_in + # Parse eagerly so an invalid version is reported already when the issue is defined + self._fixed_in_version = version.parse(fixed_in) if fixed_in else None self.message = message self.gh_issue = gh_issue.GHIssue(number=self.issue, repo=self.repo) @@ -74,10 +76,9 @@ def _issue_blocked_in_version(self, product_version: version.Version) -> bool: # The issue is blocked if it was fixed or integrated into a product version that is greater # than the product version we are currently running. - if self.fixed_in and version.parse(self.fixed_in) > product_version: # noqa:SIM103 - return True - - return False + if self._fixed_in_version is None: + return False + return self._fixed_in_version > product_version def _cli_issue_is_blocked(self) -> bool: """Check if cardano-cli issue is blocked.""" From c3b27075a5eba663083daf78db6ba24f20e4aac8 Mon Sep 17 00:00:00 2001 From: Martin Kourim Date: Mon, 3 Aug 2026 17:28:22 +0200 Subject: [PATCH 05/13] docs(blockers): fix comment typo --- cardano_node_tests/utils/blockers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cardano_node_tests/utils/blockers.py b/cardano_node_tests/utils/blockers.py index 879f20fa7..53dbeb39c 100644 --- a/cardano_node_tests/utils/blockers.py +++ b/cardano_node_tests/utils/blockers.py @@ -70,7 +70,7 @@ def _issue_blocked_in_version(self, product_version: version.Version) -> bool: msg = f"Issue '{self.repo}#{self.issue}' doesn't exist" raise ValueError(msg) - # The issue is blocked if it is was not closed yet + # The issue is blocked if it was not closed yet if state != "closed": return True From 0b90d18df7eaadc5a93c23a7d5b653fc8f0ac08e Mon Sep 17 00:00:00 2001 From: Martin Kourim Date: Mon, 3 Aug 2026 17:36:35 +0200 Subject: [PATCH 06/13] fix(gh_issue): improve issue state retrieval - Name the state sentinels (STATE_UNKNOWN, STATE_FAILURE) so callers don't have to match bare strings. - Don't cache transient retrieval failures. A single rate-limit burst or network blip no longer marks the issue as failed for the whole pytest run, and the failure is logged on every attempt. - Drop is_closed(), its last caller now works with get_state() directly. - Document get_state() return values. --- cardano_node_tests/utils/gh_issue.py | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/cardano_node_tests/utils/gh_issue.py b/cardano_node_tests/utils/gh_issue.py index bd8f2dc0a..7f1708e28 100644 --- a/cardano_node_tests/utils/gh_issue.py +++ b/cardano_node_tests/utils/gh_issue.py @@ -7,6 +7,11 @@ LOGGER = logging.getLogger(__name__) +#: State reported when the issue cannot be found, e.g. wrong issue number or repo name. +STATE_UNKNOWN: tp.Final[str] = "unknown" +#: State reported when the issue state could not be retrieved, e.g. due to an API failure. +STATE_FAILURE: tp.Final[str] = "get_state_failure" + class GHIssue: """GitHub issue.""" @@ -52,7 +57,13 @@ def url(self) -> str: return f"https://github.com/{self.repo}/issues/{self.number}" def get_state(self) -> str | None: - """Get issue state.""" + """Get issue state. + + Returns: + The issue state (e.g. "open", "closed"), `STATE_UNKNOWN` when the issue cannot + be found, `STATE_FAILURE` when the state could not be retrieved, or `None` when + the GitHub instance is not available. + """ if not self.github: LOGGER.error("Failed to get GitHub instance") return None @@ -65,17 +76,14 @@ def get_state(self) -> str | None: cached_state = self.github.get_repo(self.repo).get_issue(self.number).state.lower() except github.UnknownObjectException: LOGGER.exception("Unknown issue '%s'", identifier) - cached_state = "unknown" + cached_state = STATE_UNKNOWN except Exception: LOGGER.exception("Failed to get issue '%s'", identifier) - cached_state = "get_state_failure" + # Don't cache the failure, the retrieval may succeed on the next call + return STATE_FAILURE self.issue_cache[identifier] = cached_state return cached_state - def is_closed(self) -> bool: - """Check if issue is closed.""" - return self.get_state() == "closed" - def __repr__(self) -> str: return f"" From ade2fe01424a7f56d6cf5630a72217a9dbae555f Mon Sep 17 00:00:00 2001 From: Martin Kourim Date: Mon, 3 Aug 2026 17:36:35 +0200 Subject: [PATCH 07/13] fix(blockers): handle undetermined issue state - Log a warning when the issue state could not be determined (API failure, rate limiting, missing GitHub instance) instead of silently xfailing the test as if the issue was known to be open. - Use named state sentinels from gh_issue instead of bare strings. - Turn _fixed_in_version into a property so the blocked check stays correct when the fixed_in attribute is changed after init (the copy() + mutate pattern used with the message attribute). Keep eager validation in __init__ and include the issue reference in the error message. - Broaden the nonexistent-issue wording: UnknownObjectException is also raised for inaccessible or renamed repos. - Add missing Args/Returns/Raises docstring sections. --- cardano_node_tests/utils/blockers.py | 66 ++++++++++++++++++++++++---- 1 file changed, 57 insertions(+), 9 deletions(-) diff --git a/cardano_node_tests/utils/blockers.py b/cardano_node_tests/utils/blockers.py index 53dbeb39c..bd377c61b 100644 --- a/cardano_node_tests/utils/blockers.py +++ b/cardano_node_tests/utils/blockers.py @@ -38,8 +38,13 @@ def __init__( self.issue = issue self.repo = repo self.fixed_in = fixed_in - # Parse eagerly so an invalid version is reported already when the issue is defined - self._fixed_in_version = version.parse(fixed_in) if fixed_in else None + # Validate eagerly so an invalid version is reported already when the issue is defined + if fixed_in: + try: + version.parse(fixed_in) + except version.InvalidVersion as excp: + msg = f"Invalid `fixed_in` version for issue '{repo}#{issue}': '{fixed_in}'" + raise ValueError(msg) from excp self.message = message self.gh_issue = gh_issue.GHIssue(number=self.issue, repo=self.repo) @@ -51,8 +56,27 @@ def __init__( else: self.is_blocked = self._issue_is_blocked + @property + def _fixed_in_version(self) -> version.Version | None: + """Parsed `fixed_in` version, or `None` when no `fixed_in` was set. + + Parsed on access, so the check stays correct even when the `fixed_in` + attribute was changed after init. + """ + return version.parse(self.fixed_in) if self.fixed_in else None + def _issue_blocked_in_version(self, product_version: version.Version) -> bool: - """Check if an issue is blocked in given product version.""" + """Check if an issue is blocked in given product version. + + Args: + product_version: A version of the product to check the issue against. + + Returns: + Whether the issue is considered blocked. + + Raises: + ValueError: If the GitHub issue doesn't exist or is not accessible. + """ # Assume that the issue is blocked if no GitHub token was provided and so the check # cannot be performed. if not self.gh_issue.TOKEN: @@ -64,12 +88,21 @@ def _issue_blocked_in_version(self, product_version: version.Version) -> bool: state = self.gh_issue.get_state() - # Fail early when the issue doesn't exist, e.g. because of a typo in the issue number. - # Otherwise the test would be silently xfailed forever. - if state == "unknown": - msg = f"Issue '{self.repo}#{self.issue}' doesn't exist" + # Fail early when the issue cannot be found, e.g. because of a typo in the issue + # number or repo name. Otherwise the test would be silently xfailed forever. + if state == gh_issue.STATE_UNKNOWN: + msg = f"Issue '{self.repo}#{self.issue}' doesn't exist or is not accessible" raise ValueError(msg) + # Assume that the issue is blocked when its state could not be determined, + # e.g. due to an API failure or rate limiting. + if state is None or state == gh_issue.STATE_FAILURE: + LOGGER.warning( + "Could not determine state of issue '%s', assuming it is blocked", + f"{self.repo}#{self.issue}", + ) + return True + # The issue is blocked if it was not closed yet if state != "closed": return True @@ -93,7 +126,14 @@ def _issue_is_blocked(self) -> bool: return self._issue_blocked_in_version(VERSIONS.node) def finish_test(self, force_blocked: bool = False) -> None: - """Fail or Xfail test with GitHub issue reference.""" + """Fail or Xfail test with GitHub issue reference. + + Args: + force_blocked: Treat the issue as blocked without checking its state. + + Raises: + ValueError: If the GitHub issue doesn't exist or is not accessible. + """ reason = f"{self.gh_issue}: {self.message}" log_message = f"{self.gh_issue.url} => {self.message}" @@ -118,7 +158,15 @@ def __repr__(self) -> str: def finish_test(issues: tp.Iterable[GH]) -> None: - """Fail or Xfail test with references to multiple GitHub issues.""" + """Fail or Xfail test with references to multiple GitHub issues. + + Args: + issues: GitHub issues to report. Must not be empty. + + Raises: + ValueError: If no issues were provided, or if a referenced GitHub issue doesn't + exist or is not accessible. + """ def _get_outcome(issue: GH) -> tuple[bool, str, str]: blocked = issue.is_blocked() From 7901f6f5f16776b525af940ffc8d66ad69c82cb8 Mon Sep 17 00:00:00 2001 From: Martin Kourim Date: Mon, 3 Aug 2026 17:38:31 +0200 Subject: [PATCH 08/13] test(framework): add unit tests for blockers Cover eager fixed_in validation, the blocked check for all issue states (open, closed, unknown, undetermined), fixed_in comparison, the no-token path, and both finish_test variants. GitHub API access is mocked via GHIssue.get_state. --- framework_tests/test_blockers.py | 163 +++++++++++++++++++++++++++++++ 1 file changed, 163 insertions(+) create mode 100644 framework_tests/test_blockers.py diff --git a/framework_tests/test_blockers.py b/framework_tests/test_blockers.py new file mode 100644 index 000000000..fc587ab67 --- /dev/null +++ b/framework_tests/test_blockers.py @@ -0,0 +1,163 @@ +"""Unit tests for `cardano_node_tests.utils.blockers`. + +The tests must not touch the GitHub API. The issue state is controlled by monkeypatching +`GHIssue.get_state` and the token by monkeypatching `GHIssue.TOKEN`. +""" + +import logging + +import pytest +from packaging import version + +from cardano_node_tests.utils import blockers +from cardano_node_tests.utils import gh_issue + +PRODUCT_VERSION = version.parse("2.0.0") + + +@pytest.fixture +def gh_token(monkeypatch: pytest.MonkeyPatch): + """Set a dummy GitHub token so the blocked check is not skipped.""" + monkeypatch.setattr(gh_issue.GHIssue, "TOKEN", "dummy_token") + + +def _set_state(monkeypatch: pytest.MonkeyPatch, state: str | None) -> None: + """Make `GHIssue.get_state` return the given state without any API call.""" + monkeypatch.setattr(gh_issue.GHIssue, "get_state", lambda self: state) + + +class TestFixedIn: + """Tests for the `fixed_in` handling.""" + + def test_invalid_fixed_in(self): + """Report an invalid `fixed_in` version already when the issue is defined.""" + with pytest.raises(ValueError, match="Invalid `fixed_in` version for issue 'r/r#1'"): + blockers.GH(issue=1, repo="r/r", fixed_in="not-a-version") + + @pytest.mark.usefixtures("gh_token") + def test_fixed_in_changed_after_copy(self, monkeypatch: pytest.MonkeyPatch): + """Respect a `fixed_in` value that was changed after init.""" + _set_state(monkeypatch, "closed") + issue = blockers.GH(issue=1, fixed_in="1.0.0") + assert issue._issue_blocked_in_version(PRODUCT_VERSION) is False + + issue_copy = issue.copy() + issue_copy.fixed_in = "3.0.0" + assert issue_copy._issue_blocked_in_version(PRODUCT_VERSION) is True + + +@pytest.mark.usefixtures("gh_token") +class TestIsBlocked: + """Tests for the issue blocked check.""" + + def test_open_issue_is_blocked(self, monkeypatch: pytest.MonkeyPatch): + """Treat an open issue as blocked.""" + _set_state(monkeypatch, "open") + issue = blockers.GH(issue=1) + assert issue._issue_blocked_in_version(PRODUCT_VERSION) is True + + def test_closed_issue_is_not_blocked(self, monkeypatch: pytest.MonkeyPatch): + """Treat a closed issue without `fixed_in` as not blocked.""" + _set_state(monkeypatch, "closed") + issue = blockers.GH(issue=1) + assert issue._issue_blocked_in_version(PRODUCT_VERSION) is False + + def test_closed_fixed_in_future_version(self, monkeypatch: pytest.MonkeyPatch): + """Treat a closed issue as blocked when the fix is in a newer product version.""" + _set_state(monkeypatch, "closed") + issue = blockers.GH(issue=1, fixed_in="3.0.0") + assert issue._issue_blocked_in_version(PRODUCT_VERSION) is True + + def test_closed_fixed_in_current_version(self, monkeypatch: pytest.MonkeyPatch): + """Treat a closed issue as not blocked when the fix is in the current version.""" + _set_state(monkeypatch, "closed") + issue = blockers.GH(issue=1, fixed_in="2.0.0") + assert issue._issue_blocked_in_version(PRODUCT_VERSION) is False + + def test_nonexistent_issue(self, monkeypatch: pytest.MonkeyPatch): + """Raise an error when the issue cannot be found, instead of xfailing forever.""" + _set_state(monkeypatch, gh_issue.STATE_UNKNOWN) + issue = blockers.GH(issue=1, repo="r/r") + with pytest.raises(ValueError, match="Issue 'r/r#1' doesn't exist"): + issue._issue_blocked_in_version(PRODUCT_VERSION) + + @pytest.mark.parametrize("state", [gh_issue.STATE_FAILURE, None]) + def test_undetermined_state_is_blocked( + self, + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, + state: str | None, + ): + """Assume blocked and warn when the issue state could not be determined.""" + _set_state(monkeypatch, state) + issue = blockers.GH(issue=1) + with caplog.at_level(logging.WARNING): + assert issue._issue_blocked_in_version(PRODUCT_VERSION) is True + assert "Could not determine state" in caplog.text + + def test_no_token_is_blocked( + self, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture + ): + """Assume blocked and warn when no GitHub token is available.""" + monkeypatch.setattr(gh_issue.GHIssue, "TOKEN", None) + monkeypatch.setattr( + gh_issue.GHIssue, + "get_state", + lambda self: pytest.fail("get_state must not be called"), + ) + issue = blockers.GH(issue=1) + with caplog.at_level(logging.WARNING): + assert issue._issue_blocked_in_version(PRODUCT_VERSION) is True + assert "No GitHub token provided" in caplog.text + + +@pytest.mark.usefixtures("gh_token") +class TestFinishTest: + """Tests for `GH.finish_test` and the module-level `finish_test`.""" + + def test_blocked_issue_xfails(self, monkeypatch: pytest.MonkeyPatch): + """Xfail the test when the issue is blocked.""" + _set_state(monkeypatch, "open") + issue = blockers.GH(issue=1, message="msg1") + with pytest.raises(pytest.xfail.Exception, match="msg1"): + issue.finish_test() + + def test_unblocked_issue_fails(self, monkeypatch: pytest.MonkeyPatch): + """Fail the test when the issue is not blocked.""" + _set_state(monkeypatch, "closed") + issue = blockers.GH(issue=1, message="msg1") + with pytest.raises(pytest.fail.Exception, match="msg1"): + issue.finish_test() + + def test_force_blocked_skips_state_check(self, monkeypatch: pytest.MonkeyPatch): + """Xfail without checking the issue state when `force_blocked` is used.""" + monkeypatch.setattr( + gh_issue.GHIssue, + "get_state", + lambda self: pytest.fail("get_state must not be called"), + ) + issue = blockers.GH(issue=1, message="msg1") + with pytest.raises(pytest.xfail.Exception, match="msg1"): + issue.finish_test(force_blocked=True) + + def test_no_issues(self): + """Reject an empty issues iterable.""" + with pytest.raises(ValueError, match="No issues were provided"): + blockers.finish_test(issues=[]) + + def test_all_blocked_xfails(self, monkeypatch: pytest.MonkeyPatch): + """Xfail the test when all issues are blocked.""" + _set_state(monkeypatch, "open") + issues = [blockers.GH(issue=1, message="msg1"), blockers.GH(issue=2, message="msg2")] + with pytest.raises(pytest.xfail.Exception, match="msg1.*msg2"): + blockers.finish_test(issues=issues) + + def test_some_unblocked_fails(self, monkeypatch: pytest.MonkeyPatch): + """Fail the test when at least one issue is not blocked.""" + states = {1: "open", 2: "closed"} + monkeypatch.setattr( + gh_issue.GHIssue, "get_state", lambda self: states[self.number] + ) + issues = [blockers.GH(issue=1, message="msg1"), blockers.GH(issue=2, message="msg2")] + with pytest.raises(pytest.fail.Exception, match="XFAIL.*msg1.*FAIL.*msg2"): + blockers.finish_test(issues=issues) From d77e76759490e9bc9f0fe7ff974f2c699875ffd2 Mon Sep 17 00:00:00 2001 From: Martin Kourim Date: Mon, 3 Aug 2026 17:43:00 +0200 Subject: [PATCH 09/13] refactor(gh_issue): always return str from get_state An unavailable GitHub instance and a failed state retrieval are handled identically by the only caller, so collapse the None signal into STATE_FAILURE. Also add STATE_CLOSED so no caller needs a bare state string. --- cardano_node_tests/utils/gh_issue.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/cardano_node_tests/utils/gh_issue.py b/cardano_node_tests/utils/gh_issue.py index 7f1708e28..814256fe3 100644 --- a/cardano_node_tests/utils/gh_issue.py +++ b/cardano_node_tests/utils/gh_issue.py @@ -7,6 +7,8 @@ LOGGER = logging.getLogger(__name__) +#: State of a closed issue. +STATE_CLOSED: tp.Final[str] = "closed" #: State reported when the issue cannot be found, e.g. wrong issue number or repo name. STATE_UNKNOWN: tp.Final[str] = "unknown" #: State reported when the issue state could not be retrieved, e.g. due to an API failure. @@ -56,17 +58,16 @@ def github(self) -> github.Github | None: def url(self) -> str: return f"https://github.com/{self.repo}/issues/{self.number}" - def get_state(self) -> str | None: + def get_state(self) -> str: """Get issue state. Returns: - The issue state (e.g. "open", "closed"), `STATE_UNKNOWN` when the issue cannot - be found, `STATE_FAILURE` when the state could not be retrieved, or `None` when - the GitHub instance is not available. + The issue state (e.g. "open", `STATE_CLOSED`), `STATE_UNKNOWN` when the issue + cannot be found, or `STATE_FAILURE` when the state could not be retrieved. """ if not self.github: LOGGER.error("Failed to get GitHub instance") - return None + return STATE_FAILURE identifier = f"{self.repo}#{self.number}" cached_state = self.issue_cache.get(identifier) From 6a71ad26cbce06e1ecf5facd673051e30d0e362b Mon Sep 17 00:00:00 2001 From: Martin Kourim Date: Mon, 3 Aug 2026 17:46:44 +0200 Subject: [PATCH 10/13] refactor(blockers): simplify state and version checks - Drop the _fixed_in_version property, parse fixed_in inline at its single use site. Validation stays in __init__ and post-init changes to fixed_in keep being respected. - Keep the __init__ attribute assignments contiguous by validating before assigning. - Build the repeated repo#issue string once per check. - Use gh_issue.STATE_CLOSED instead of a bare string and drop the None handling, get_state now always returns str. - Check for empty issues before any GitHub API call and take tp.Collection instead of tp.Iterable. - Document that both finish_test variants never return. - Adjust tests: shared helpers for state patching, dispatch test for the repo to version mapping, lint fixes. --- cardano_node_tests/utils/blockers.py | 47 ++++++++-------- framework_tests/test_blockers.py | 81 +++++++++++++++------------- 2 files changed, 67 insertions(+), 61 deletions(-) diff --git a/cardano_node_tests/utils/blockers.py b/cardano_node_tests/utils/blockers.py index bd377c61b..efe9b6df2 100644 --- a/cardano_node_tests/utils/blockers.py +++ b/cardano_node_tests/utils/blockers.py @@ -35,9 +35,6 @@ def __init__( fixed_in: str = "", message: str = "", ) -> None: - self.issue = issue - self.repo = repo - self.fixed_in = fixed_in # Validate eagerly so an invalid version is reported already when the issue is defined if fixed_in: try: @@ -45,6 +42,10 @@ def __init__( except version.InvalidVersion as excp: msg = f"Invalid `fixed_in` version for issue '{repo}#{issue}': '{fixed_in}'" raise ValueError(msg) from excp + + self.issue = issue + self.repo = repo + self.fixed_in = fixed_in self.message = message self.gh_issue = gh_issue.GHIssue(number=self.issue, repo=self.repo) @@ -56,15 +57,6 @@ def __init__( else: self.is_blocked = self._issue_is_blocked - @property - def _fixed_in_version(self) -> version.Version | None: - """Parsed `fixed_in` version, or `None` when no `fixed_in` was set. - - Parsed on access, so the check stays correct even when the `fixed_in` - attribute was changed after init. - """ - return version.parse(self.fixed_in) if self.fixed_in else None - def _issue_blocked_in_version(self, product_version: version.Version) -> bool: """Check if an issue is blocked in given product version. @@ -77,12 +69,13 @@ def _issue_blocked_in_version(self, product_version: version.Version) -> bool: Raises: ValueError: If the GitHub issue doesn't exist or is not accessible. """ + issue_id = f"{self.repo}#{self.issue}" + # Assume that the issue is blocked if no GitHub token was provided and so the check # cannot be performed. if not self.gh_issue.TOKEN: LOGGER.warning( - "No GitHub token provided, cannot check if issue '%s' is blocked", - f"{self.repo}#{self.issue}", + "No GitHub token provided, cannot check if issue '%s' is blocked", issue_id ) return True @@ -91,27 +84,26 @@ def _issue_blocked_in_version(self, product_version: version.Version) -> bool: # Fail early when the issue cannot be found, e.g. because of a typo in the issue # number or repo name. Otherwise the test would be silently xfailed forever. if state == gh_issue.STATE_UNKNOWN: - msg = f"Issue '{self.repo}#{self.issue}' doesn't exist or is not accessible" + msg = f"Issue '{issue_id}' doesn't exist or is not accessible" raise ValueError(msg) # Assume that the issue is blocked when its state could not be determined, # e.g. due to an API failure or rate limiting. - if state is None or state == gh_issue.STATE_FAILURE: + if state == gh_issue.STATE_FAILURE: LOGGER.warning( - "Could not determine state of issue '%s', assuming it is blocked", - f"{self.repo}#{self.issue}", + "Could not determine state of issue '%s', assuming it is blocked", issue_id ) return True # The issue is blocked if it was not closed yet - if state != "closed": + if state != gh_issue.STATE_CLOSED: return True # The issue is blocked if it was fixed or integrated into a product version that is greater # than the product version we are currently running. - if self._fixed_in_version is None: + if not self.fixed_in: return False - return self._fixed_in_version > product_version + return version.parse(self.fixed_in) > product_version def _cli_issue_is_blocked(self) -> bool: """Check if cardano-cli issue is blocked.""" @@ -128,11 +120,14 @@ def _issue_is_blocked(self) -> bool: def finish_test(self, force_blocked: bool = False) -> None: """Fail or Xfail test with GitHub issue reference. + Never returns - the test outcome is always set via `pytest.xfail` or `pytest.fail`. + Args: force_blocked: Treat the issue as blocked without checking its state. Raises: ValueError: If the GitHub issue doesn't exist or is not accessible. + Cannot happen with `force_blocked`, as the issue state is not checked. """ reason = f"{self.gh_issue}: {self.message}" log_message = f"{self.gh_issue.url} => {self.message}" @@ -157,9 +152,11 @@ def __repr__(self) -> str: return f"" -def finish_test(issues: tp.Iterable[GH]) -> None: +def finish_test(issues: tp.Collection[GH]) -> None: """Fail or Xfail test with references to multiple GitHub issues. + Never returns - the test outcome is always set via `pytest.xfail` or `pytest.fail`. + Args: issues: GitHub issues to report. Must not be empty. @@ -167,6 +164,9 @@ def finish_test(issues: tp.Iterable[GH]) -> None: ValueError: If no issues were provided, or if a referenced GitHub issue doesn't exist or is not accessible. """ + if not issues: + msg = "No issues were provided" + raise ValueError(msg) def _get_outcome(issue: GH) -> tuple[bool, str, str]: blocked = issue.is_blocked() @@ -176,9 +176,6 @@ def _get_outcome(issue: GH) -> tuple[bool, str, str]: return blocked, reason, log_message outcomes = [_get_outcome(i) for i in issues] - if not outcomes: - msg = "No issues were provided" - raise ValueError(msg) should_fail = False for blocked, __, log_message in outcomes: diff --git a/framework_tests/test_blockers.py b/framework_tests/test_blockers.py index fc587ab67..38027d0a7 100644 --- a/framework_tests/test_blockers.py +++ b/framework_tests/test_blockers.py @@ -4,8 +4,6 @@ `GHIssue.get_state` and the token by monkeypatching `GHIssue.TOKEN`. """ -import logging - import pytest from packaging import version @@ -21,9 +19,18 @@ def gh_token(monkeypatch: pytest.MonkeyPatch): monkeypatch.setattr(gh_issue.GHIssue, "TOKEN", "dummy_token") -def _set_state(monkeypatch: pytest.MonkeyPatch, state: str | None) -> None: +def _set_state(monkeypatch: pytest.MonkeyPatch, state: str) -> None: """Make `GHIssue.get_state` return the given state without any API call.""" - monkeypatch.setattr(gh_issue.GHIssue, "get_state", lambda self: state) + monkeypatch.setattr(gh_issue.GHIssue, "get_state", lambda *_a, **_kw: state) + + +def _forbid_get_state(monkeypatch: pytest.MonkeyPatch) -> None: + """Make any `GHIssue.get_state` call fail the test.""" + monkeypatch.setattr( + gh_issue.GHIssue, + "get_state", + lambda *_a, **_kw: pytest.fail("get_state must not be called"), + ) class TestFixedIn: @@ -37,7 +44,7 @@ def test_invalid_fixed_in(self): @pytest.mark.usefixtures("gh_token") def test_fixed_in_changed_after_copy(self, monkeypatch: pytest.MonkeyPatch): """Respect a `fixed_in` value that was changed after init.""" - _set_state(monkeypatch, "closed") + _set_state(monkeypatch, gh_issue.STATE_CLOSED) issue = blockers.GH(issue=1, fixed_in="1.0.0") assert issue._issue_blocked_in_version(PRODUCT_VERSION) is False @@ -46,6 +53,24 @@ def test_fixed_in_changed_after_copy(self, monkeypatch: pytest.MonkeyPatch): assert issue_copy._issue_blocked_in_version(PRODUCT_VERSION) is True +class TestDispatch: + """Tests for the `is_blocked` dispatch based on repo.""" + + @pytest.mark.parametrize( + ("repo", "expected"), + [ + ("IntersectMBO/cardano-cli", "_cli_issue_is_blocked"), + ("IntersectMBO/cardano-db-sync", "_dbsync_issue_is_blocked"), + ("IntersectMBO/cardano-node", "_issue_is_blocked"), + ("IntersectMBO/ouroboros-consensus", "_issue_is_blocked"), + ], + ) + def test_is_blocked_dispatch(self, repo: str, expected: str): + """Select the version check that corresponds to the repo.""" + issue = blockers.GH(issue=1, repo=repo) + assert issue.is_blocked.__name__ == expected + + @pytest.mark.usefixtures("gh_token") class TestIsBlocked: """Tests for the issue blocked check.""" @@ -58,19 +83,19 @@ def test_open_issue_is_blocked(self, monkeypatch: pytest.MonkeyPatch): def test_closed_issue_is_not_blocked(self, monkeypatch: pytest.MonkeyPatch): """Treat a closed issue without `fixed_in` as not blocked.""" - _set_state(monkeypatch, "closed") + _set_state(monkeypatch, gh_issue.STATE_CLOSED) issue = blockers.GH(issue=1) assert issue._issue_blocked_in_version(PRODUCT_VERSION) is False def test_closed_fixed_in_future_version(self, monkeypatch: pytest.MonkeyPatch): """Treat a closed issue as blocked when the fix is in a newer product version.""" - _set_state(monkeypatch, "closed") + _set_state(monkeypatch, gh_issue.STATE_CLOSED) issue = blockers.GH(issue=1, fixed_in="3.0.0") assert issue._issue_blocked_in_version(PRODUCT_VERSION) is True def test_closed_fixed_in_current_version(self, monkeypatch: pytest.MonkeyPatch): """Treat a closed issue as not blocked when the fix is in the current version.""" - _set_state(monkeypatch, "closed") + _set_state(monkeypatch, gh_issue.STATE_CLOSED) issue = blockers.GH(issue=1, fixed_in="2.0.0") assert issue._issue_blocked_in_version(PRODUCT_VERSION) is False @@ -81,18 +106,13 @@ def test_nonexistent_issue(self, monkeypatch: pytest.MonkeyPatch): with pytest.raises(ValueError, match="Issue 'r/r#1' doesn't exist"): issue._issue_blocked_in_version(PRODUCT_VERSION) - @pytest.mark.parametrize("state", [gh_issue.STATE_FAILURE, None]) def test_undetermined_state_is_blocked( - self, - monkeypatch: pytest.MonkeyPatch, - caplog: pytest.LogCaptureFixture, - state: str | None, + self, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture ): """Assume blocked and warn when the issue state could not be determined.""" - _set_state(monkeypatch, state) + _set_state(monkeypatch, gh_issue.STATE_FAILURE) issue = blockers.GH(issue=1) - with caplog.at_level(logging.WARNING): - assert issue._issue_blocked_in_version(PRODUCT_VERSION) is True + assert issue._issue_blocked_in_version(PRODUCT_VERSION) is True assert "Could not determine state" in caplog.text def test_no_token_is_blocked( @@ -100,14 +120,9 @@ def test_no_token_is_blocked( ): """Assume blocked and warn when no GitHub token is available.""" monkeypatch.setattr(gh_issue.GHIssue, "TOKEN", None) - monkeypatch.setattr( - gh_issue.GHIssue, - "get_state", - lambda self: pytest.fail("get_state must not be called"), - ) + _forbid_get_state(monkeypatch) issue = blockers.GH(issue=1) - with caplog.at_level(logging.WARNING): - assert issue._issue_blocked_in_version(PRODUCT_VERSION) is True + assert issue._issue_blocked_in_version(PRODUCT_VERSION) is True assert "No GitHub token provided" in caplog.text @@ -124,24 +139,20 @@ def test_blocked_issue_xfails(self, monkeypatch: pytest.MonkeyPatch): def test_unblocked_issue_fails(self, monkeypatch: pytest.MonkeyPatch): """Fail the test when the issue is not blocked.""" - _set_state(monkeypatch, "closed") + _set_state(monkeypatch, gh_issue.STATE_CLOSED) issue = blockers.GH(issue=1, message="msg1") with pytest.raises(pytest.fail.Exception, match="msg1"): issue.finish_test() def test_force_blocked_skips_state_check(self, monkeypatch: pytest.MonkeyPatch): """Xfail without checking the issue state when `force_blocked` is used.""" - monkeypatch.setattr( - gh_issue.GHIssue, - "get_state", - lambda self: pytest.fail("get_state must not be called"), - ) + _forbid_get_state(monkeypatch) issue = blockers.GH(issue=1, message="msg1") with pytest.raises(pytest.xfail.Exception, match="msg1"): issue.finish_test(force_blocked=True) def test_no_issues(self): - """Reject an empty issues iterable.""" + """Reject an empty issues collection.""" with pytest.raises(ValueError, match="No issues were provided"): blockers.finish_test(issues=[]) @@ -149,15 +160,13 @@ def test_all_blocked_xfails(self, monkeypatch: pytest.MonkeyPatch): """Xfail the test when all issues are blocked.""" _set_state(monkeypatch, "open") issues = [blockers.GH(issue=1, message="msg1"), blockers.GH(issue=2, message="msg2")] - with pytest.raises(pytest.xfail.Exception, match="msg1.*msg2"): + with pytest.raises(pytest.xfail.Exception, match=r"msg1.*msg2"): blockers.finish_test(issues=issues) def test_some_unblocked_fails(self, monkeypatch: pytest.MonkeyPatch): """Fail the test when at least one issue is not blocked.""" - states = {1: "open", 2: "closed"} - monkeypatch.setattr( - gh_issue.GHIssue, "get_state", lambda self: states[self.number] - ) + states = {1: "open", 2: gh_issue.STATE_CLOSED} + monkeypatch.setattr(gh_issue.GHIssue, "get_state", lambda self: states[self.number]) issues = [blockers.GH(issue=1, message="msg1"), blockers.GH(issue=2, message="msg2")] - with pytest.raises(pytest.fail.Exception, match="XFAIL.*msg1.*FAIL.*msg2"): + with pytest.raises(pytest.fail.Exception, match=r"XFAIL.*msg1.*FAIL.*msg2"): blockers.finish_test(issues=issues) From 63cb52fb32033c53d38273c5d390911d505f57da Mon Sep 17 00:00:00 2001 From: Martin Kourim Date: Mon, 3 Aug 2026 17:46:44 +0200 Subject: [PATCH 11/13] test(framework): add unit tests for gh_issue caching Cover the caching semantics of get_state: real states and unknown issues are cached, transient retrieval failures are not, and a missing GitHub instance reports STATE_FAILURE. --- framework_tests/test_gh_issue.py | 80 ++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 framework_tests/test_gh_issue.py diff --git a/framework_tests/test_gh_issue.py b/framework_tests/test_gh_issue.py new file mode 100644 index 000000000..3ae8ed9f6 --- /dev/null +++ b/framework_tests/test_gh_issue.py @@ -0,0 +1,80 @@ +"""Unit tests for `cardano_node_tests.utils.gh_issue`. + +The tests must not touch the GitHub API. The `GHIssue._get_github` classmethod is +monkeypatched with a fake that returns canned issue states. +""" + +import typing as tp + +import github +import pytest + +from cardano_node_tests.utils import gh_issue + + +class _FakeGithub: + """Fake `github.Github` returning canned issue states and counting API calls.""" + + def __init__(self, responses: tp.Sequence[str | Exception]) -> None: + self.responses = list(responses) + self.calls = 0 + + def get_repo(self, _repo: str) -> "_FakeGithub": + return self + + def get_issue(self, _number: int) -> tp.Any: + self.calls += 1 + response = self.responses.pop(0) + if isinstance(response, Exception): + raise response + return type("FakeIssue", (), {"state": response}) + + +@pytest.fixture +def clean_cache(monkeypatch: pytest.MonkeyPatch): + """Give each test a fresh issue cache.""" + monkeypatch.setattr(gh_issue.GHIssue, "issue_cache", {}) + + +def _set_github(monkeypatch: pytest.MonkeyPatch, fake: _FakeGithub | None) -> None: + """Make `GHIssue` use the given fake GitHub instance.""" + monkeypatch.setattr(gh_issue.GHIssue, "_get_github", classmethod(lambda _cls: fake)) + + +@pytest.mark.usefixtures("clean_cache") +class TestGetState: + """Tests for `GHIssue.get_state`.""" + + def test_state_cached(self, monkeypatch: pytest.MonkeyPatch): + """Retrieve the state once and serve subsequent calls from the cache.""" + fake = _FakeGithub(responses=["CLOSED"]) + _set_github(monkeypatch, fake) + issue = gh_issue.GHIssue(number=1, repo="r/r") + assert issue.get_state() == gh_issue.STATE_CLOSED + assert issue.get_state() == gh_issue.STATE_CLOSED + assert fake.calls == 1 + + def test_unknown_issue_cached(self, monkeypatch: pytest.MonkeyPatch): + """Cache the state of a nonexistent issue, it cannot appear later.""" + fake = _FakeGithub(responses=[github.UnknownObjectException(status=404)]) + _set_github(monkeypatch, fake) + issue = gh_issue.GHIssue(number=1, repo="r/r") + assert issue.get_state() == gh_issue.STATE_UNKNOWN + assert issue.get_state() == gh_issue.STATE_UNKNOWN + assert fake.calls == 1 + + def test_transient_failure_not_cached(self, monkeypatch: pytest.MonkeyPatch): + """Don't cache a failed state retrieval, the next call may succeed.""" + fake = _FakeGithub(responses=[RuntimeError("API is down"), "OPEN"]) + _set_github(monkeypatch, fake) + issue = gh_issue.GHIssue(number=1, repo="r/r") + assert issue.get_state() == gh_issue.STATE_FAILURE + assert not gh_issue.GHIssue.issue_cache + assert issue.get_state() == "open" + assert fake.calls == 2 + + def test_no_github_instance(self, monkeypatch: pytest.MonkeyPatch): + """Report a state retrieval failure when the GitHub instance is not available.""" + _set_github(monkeypatch, None) + issue = gh_issue.GHIssue(number=1, repo="r/r") + assert issue.get_state() == gh_issue.STATE_FAILURE From e6fc7ef4a17af494ebded1def210691e06fffb1c Mon Sep 17 00:00:00 2001 From: Martin Kourim Date: Mon, 3 Aug 2026 17:49:46 +0200 Subject: [PATCH 12/13] style: polish gh_issue constants and fake - Use plain comments instead of the Sphinx #: marker, which is not used anywhere else in the codebase. - Clarify get_state Returns wording. - Use types.SimpleNamespace and an iterator in the fake GitHub helper instead of a dynamic class and list.pop bookkeeping. --- cardano_node_tests/utils/gh_issue.py | 11 ++++++----- framework_tests/test_gh_issue.py | 11 ++++++----- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/cardano_node_tests/utils/gh_issue.py b/cardano_node_tests/utils/gh_issue.py index 814256fe3..90bae7d40 100644 --- a/cardano_node_tests/utils/gh_issue.py +++ b/cardano_node_tests/utils/gh_issue.py @@ -7,11 +7,11 @@ LOGGER = logging.getLogger(__name__) -#: State of a closed issue. +# State of a closed issue. STATE_CLOSED: tp.Final[str] = "closed" -#: State reported when the issue cannot be found, e.g. wrong issue number or repo name. +# State reported when the issue cannot be found, e.g. wrong issue number or repo name. STATE_UNKNOWN: tp.Final[str] = "unknown" -#: State reported when the issue state could not be retrieved, e.g. due to an API failure. +# State reported when the issue state could not be retrieved, e.g. due to an API failure. STATE_FAILURE: tp.Final[str] = "get_state_failure" @@ -62,8 +62,9 @@ def get_state(self) -> str: """Get issue state. Returns: - The issue state (e.g. "open", `STATE_CLOSED`), `STATE_UNKNOWN` when the issue - cannot be found, or `STATE_FAILURE` when the state could not be retrieved. + The state as reported by GitHub, e.g. "open" or `STATE_CLOSED`; `STATE_UNKNOWN` + when the issue cannot be found, or `STATE_FAILURE` when the state could not + be retrieved. """ if not self.github: LOGGER.error("Failed to get GitHub instance") diff --git a/framework_tests/test_gh_issue.py b/framework_tests/test_gh_issue.py index 3ae8ed9f6..d5799ff80 100644 --- a/framework_tests/test_gh_issue.py +++ b/framework_tests/test_gh_issue.py @@ -4,6 +4,7 @@ monkeypatched with a fake that returns canned issue states. """ +import types import typing as tp import github @@ -16,18 +17,18 @@ class _FakeGithub: """Fake `github.Github` returning canned issue states and counting API calls.""" def __init__(self, responses: tp.Sequence[str | Exception]) -> None: - self.responses = list(responses) + self.responses = iter(responses) self.calls = 0 - def get_repo(self, _repo: str) -> "_FakeGithub": + def get_repo(self, _repo: str) -> tp.Self: return self - def get_issue(self, _number: int) -> tp.Any: + def get_issue(self, _number: int) -> types.SimpleNamespace: self.calls += 1 - response = self.responses.pop(0) + response = next(self.responses) if isinstance(response, Exception): raise response - return type("FakeIssue", (), {"state": response}) + return types.SimpleNamespace(state=response) @pytest.fixture From fb82ef9c3d72f0b7f09861289d0ba2e546454f9a Mon Sep 17 00:00:00 2001 From: Martin Kourim Date: Mon, 3 Aug 2026 18:29:29 +0200 Subject: [PATCH 13/13] style(framework): annotate fixture return types Fixtures in framework_tests consistently annotate the return type, only tests omit the None return type. --- framework_tests/test_blockers.py | 2 +- framework_tests/test_gh_issue.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/framework_tests/test_blockers.py b/framework_tests/test_blockers.py index 38027d0a7..e4c5d64b1 100644 --- a/framework_tests/test_blockers.py +++ b/framework_tests/test_blockers.py @@ -14,7 +14,7 @@ @pytest.fixture -def gh_token(monkeypatch: pytest.MonkeyPatch): +def gh_token(monkeypatch: pytest.MonkeyPatch) -> None: """Set a dummy GitHub token so the blocked check is not skipped.""" monkeypatch.setattr(gh_issue.GHIssue, "TOKEN", "dummy_token") diff --git a/framework_tests/test_gh_issue.py b/framework_tests/test_gh_issue.py index d5799ff80..2112a5662 100644 --- a/framework_tests/test_gh_issue.py +++ b/framework_tests/test_gh_issue.py @@ -32,7 +32,7 @@ def get_issue(self, _number: int) -> types.SimpleNamespace: @pytest.fixture -def clean_cache(monkeypatch: pytest.MonkeyPatch): +def clean_cache(monkeypatch: pytest.MonkeyPatch) -> None: """Give each test a fresh issue cache.""" monkeypatch.setattr(gh_issue.GHIssue, "issue_cache", {})