From bb83f97a6efd5896a35aa41ade4951c1be16d350 Mon Sep 17 00:00:00 2001 From: German <28149841+germa89@users.noreply.github.com> Date: Thu, 22 May 2025 14:29:29 +0200 Subject: [PATCH 1/6] refactor: enhance error reporting in pytest output for better debugging --- tests/conftest.py | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 991257c6f7..0ba3b26843 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -20,6 +20,7 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE # SOFTWARE. +import ast from collections import namedtuple from collections.abc import Generator import os @@ -316,17 +317,38 @@ def short_test_summary(self): # your own impl goes here, for example: self.write_sep("=", "PyMAPDL Pytest short summary") + def get_error_message(rep): + rep_ = rep.longreprtext.splitlines() + location = rep.location + path = f"{location[0]}:{location[1]}" + if len(rep_) >= 3: + # It is a fail/error + # A list of all the lines of the failed test + an empty string + # and the test location. + err_type = rep_[-1].split(":")[-1].strip() + cause = rep_[-3] # Picking the last line of the error message + cause = cause[2:].strip() if cause.startswith("E ") else cause.strip() + return f"{path} - {err_type}: {cause}" + else: + # Skip rep_ is a list with on string + tupl_ = ast.literal_eval(rep_[0]) + if len(tupl_) < 3: + # Early exit just in case + return f"{path} - " + " ".join(tupl_) + cause = f"{tupl_[2]}" + return f"{path} - {cause}" + failed = self.stats.get("failed", []) for rep in failed: - self.write_line( - f"[FAILED] {rep.head_line} - {rep.longreprtext.splitlines()[-3]}" - ) + self.write_line(f"[FAILED] {rep.head_line} - {get_error_message(rep)}") + + skipped = self.stats.get("skipped", []) + for rep in skipped: + self.write_line(f"[SKIPPED] {rep.head_line} - {get_error_message(rep)}") errored = self.stats.get("error", []) for rep in errored: - self.write_line( - f"[ERROR] {rep.head_line} - {rep.longreprtext.splitlines()[-3]}" - ) + self.write_line(f"[ERROR] {rep.head_line} - {get_error_message(rep)}") @pytest.hookimpl(trylast=True) From cd8c199d66b09a91383d7571ff100b0acfd9308d Mon Sep 17 00:00:00 2001 From: pyansys-ci-bot <92810346+pyansys-ci-bot@users.noreply.github.com> Date: Thu, 22 May 2025 12:33:36 +0000 Subject: [PATCH 2/6] chore: adding changelog file 3943.added.md [dependabot-skip] --- doc/changelog.d/3943.added.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 doc/changelog.d/3943.added.md diff --git a/doc/changelog.d/3943.added.md b/doc/changelog.d/3943.added.md new file mode 100644 index 0000000000..7460dafca2 --- /dev/null +++ b/doc/changelog.d/3943.added.md @@ -0,0 +1 @@ +refactor: enhance error reporting in pytest output for better debugging \ No newline at end of file From aca6d724ace26ec7302429bf8e7d6077e3a076c9 Mon Sep 17 00:00:00 2001 From: German <28149841+germa89@users.noreply.github.com> Date: Thu, 22 May 2025 14:51:31 +0200 Subject: [PATCH 3/6] refactor: avoid using ast --- tests/conftest.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 0ba3b26843..bfabd738a7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -20,7 +20,6 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE # SOFTWARE. -import ast from collections import namedtuple from collections.abc import Generator import os @@ -321,6 +320,7 @@ def get_error_message(rep): rep_ = rep.longreprtext.splitlines() location = rep.location path = f"{location[0]}:{location[1]}" + if len(rep_) >= 3: # It is a fail/error # A list of all the lines of the failed test + an empty string @@ -331,11 +331,10 @@ def get_error_message(rep): return f"{path} - {err_type}: {cause}" else: # Skip rep_ is a list with on string - tupl_ = ast.literal_eval(rep_[0]) - if len(tupl_) < 3: - # Early exit just in case - return f"{path} - " + " ".join(tupl_) - cause = f"{tupl_[2]}" + if len(rep.longrepr) < 2: + cause = rep.longrepr[-1] + else: + cause = rep.longrepr[2] return f"{path} - {cause}" failed = self.stats.get("failed", []) From 46a885d8c1094b918e1eb708deaf85741f943ad4 Mon Sep 17 00:00:00 2001 From: German <28149841+germa89@users.noreply.github.com> Date: Thu, 22 May 2025 15:14:33 +0200 Subject: [PATCH 4/6] refactor: enhance output when skipping a full module --- tests/conftest.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index bfabd738a7..469bae0a2f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -319,7 +319,13 @@ def short_test_summary(self): def get_error_message(rep): rep_ = rep.longreprtext.splitlines() location = rep.location - path = f"{location[0]}:{location[1]}" + sep = " - " + if location[0] == location[2]: + # Skipping a module. + path = "" # already included as head_title + sep = "" + else: + path = f"{location[0]}:{location[1]}" if len(rep_) >= 3: # It is a fail/error @@ -328,14 +334,14 @@ def get_error_message(rep): err_type = rep_[-1].split(":")[-1].strip() cause = rep_[-3] # Picking the last line of the error message cause = cause[2:].strip() if cause.startswith("E ") else cause.strip() - return f"{path} - {err_type}: {cause}" + return f"{path}{sep}{err_type}: {cause}" else: # Skip rep_ is a list with on string if len(rep.longrepr) < 2: cause = rep.longrepr[-1] else: cause = rep.longrepr[2] - return f"{path} - {cause}" + return f"{path}{sep}{cause}" failed = self.stats.get("failed", []) for rep in failed: From b0ec215c22050598989b2fbdf1c0a2adb6ac1cbb Mon Sep 17 00:00:00 2001 From: German <28149841+germa89@users.noreply.github.com> Date: Thu, 22 May 2025 17:42:07 +0200 Subject: [PATCH 5/6] refactor: improve test result reporting with enhanced message formatting --- tests/conftest.py | 103 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 79 insertions(+), 24 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 469bae0a2f..62bc488569 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -315,51 +315,106 @@ class MyReporter(TerminalReporter): def short_test_summary(self): # your own impl goes here, for example: self.write_sep("=", "PyMAPDL Pytest short summary") + markup = self._tw.markup - def get_error_message(rep): - rep_ = rep.longreprtext.splitlines() + if self.hasmarkup: + color = True + else: + color = False + + ERROR_COLOR = {"Red": color, "bold": True} + FAILED_COLOR = {"red": color, "bold": True} + PASSED_COLOR = {"green": color} + SKIPPED_COLOR = {"green": color, "bold": True} + XPASSED_COLOR = {"Yellow": color, "bold": True} + XFAILED_COLOR = {"yellow": color} + + # self._tw.markup("asdf", Red=True) + + def get_normal_message(rep, header, message): location = rep.location - sep = " - " + if message: + message = f" - {message}" + if location[0] == location[2]: - # Skipping a module. - path = "" # already included as head_title - sep = "" + return f"{header} {rep.head_line}{message}" else: path = f"{location[0]}:{location[1]}" + return f"{header} {rep.head_line} - {path}{message}" - if len(rep_) >= 3: - # It is a fail/error - # A list of all the lines of the failed test + an empty string - # and the test location. - err_type = rep_[-1].split(":")[-1].strip() - cause = rep_[-3] # Picking the last line of the error message - cause = cause[2:].strip() if cause.startswith("E ") else cause.strip() - return f"{path}{sep}{err_type}: {cause}" - else: - # Skip rep_ is a list with on string - if len(rep.longrepr) < 2: - cause = rep.longrepr[-1] - else: - cause = rep.longrepr[2] - return f"{path}{sep}{cause}" + def get_failure_message(rep, header, message): + location = rep.location + path = f"{location[0]}:{location[1]}" + cause = message.splitlines() + cause = " ".join( + [ + each[2:].strip() if each.startswith("E ") else each.strip() + for each in cause + ] + ) + + return f"{header} {rep.head_line} - {path}: {cause}" + + def get_skip_message(rep): + message = rep.longrepr[2] + header = markup("[SKIPPED]", **SKIPPED_COLOR) + return get_normal_message(rep, header, message) + + def get_passed_message(rep): + message = rep.longreprtext + header = markup("[PASSED]", **PASSED_COLOR) + return get_normal_message(rep, header, message) + + def get_xfailed_message(rep): + message = " ".join(rep.longrepr.reprcrash.message.split(":")[1:]).strip() + header = markup("[XFAILED]", **XFAILED_COLOR) + return get_normal_message(rep, header, message) + + def get_xpassed_message(rep): + message = rep.longreprtext + header = markup("[XPASSED]", **XPASSED_COLOR) + return get_normal_message(rep, header, message) + + def get_error_message(rep): + message = rep.longrepr.reprcrash.message + header = markup("[ERROR]", **ERROR_COLOR) + return get_failure_message(rep, header, message) + + def get_failed_message(rep): + message = rep.longrepr.reprcrash.message + header = markup("[FAILED]", **FAILED_COLOR) + return get_failure_message(rep, header, message) failed = self.stats.get("failed", []) for rep in failed: - self.write_line(f"[FAILED] {rep.head_line} - {get_error_message(rep)}") + self.write_line(get_failed_message(rep)) skipped = self.stats.get("skipped", []) for rep in skipped: - self.write_line(f"[SKIPPED] {rep.head_line} - {get_error_message(rep)}") + self.write_line(get_skip_message(rep)) errored = self.stats.get("error", []) for rep in errored: - self.write_line(f"[ERROR] {rep.head_line} - {get_error_message(rep)}") + self.write_line(get_error_message(rep)) + + passed = self.stats.get("passed", []) + for rep in passed: + self.write_line(get_passed_message(rep)) + + xpassed = self.stats.get("xpassed", []) + for rep in xpassed: + self.write_line(get_xpassed_message(rep)) + + xfailed = self.stats.get("xfailed", []) + for rep in xfailed: + self.write_line(get_xfailed_message(rep)) @pytest.hookimpl(trylast=True) def pytest_configure(config): vanilla_reporter = config.pluginmanager.getplugin("terminalreporter") my_reporter = MyReporter(config) + my_reporter._tw.fullwidth = 160 config.pluginmanager.unregister(vanilla_reporter) config.pluginmanager.register(my_reporter, "terminalreporter") From ccf493018a89fbfead7beb02ffa322de3bab0152 Mon Sep 17 00:00:00 2001 From: German <28149841+germa89@users.noreply.github.com> Date: Thu, 22 May 2025 17:43:44 +0200 Subject: [PATCH 6/6] refactor: enhance pytest skip messages for missing dependencies --- tests/test_krylov.py | 5 ++++- tests/test_launcher_remote.py | 5 ++++- tests/test_plotting.py | 4 +++- tests/test_pool.py | 4 +++- tests/test_theme.py | 4 +++- 5 files changed, 17 insertions(+), 5 deletions(-) diff --git a/tests/test_krylov.py b/tests/test_krylov.py index 9a3c83c456..898d7a2144 100644 --- a/tests/test_krylov.py +++ b/tests/test_krylov.py @@ -31,7 +31,10 @@ if not has_dependency("ansys-math-core"): # Needs ansys-math-core - pytest.skip(allow_module_level=True) + pytest.skip( + allow_module_level=True, + reason="Skipping because 'ansys-math-core' is not installed", + ) PATH = os.path.dirname(os.path.abspath(__file__)) diff --git a/tests/test_launcher_remote.py b/tests/test_launcher_remote.py index fa5e937085..1b7e2d10ee 100644 --- a/tests/test_launcher_remote.py +++ b/tests/test_launcher_remote.py @@ -26,7 +26,10 @@ from conftest import has_dependency if not has_dependency("ansys-platform-instancemanagement"): - pytest.skip(allow_module_level=True) + pytest.skip( + allow_module_level=True, + reason="Skipping because 'ansys-platform-instancemanagement' is not installed", + ) from unittest.mock import create_autospec diff --git a/tests/test_plotting.py b/tests/test_plotting.py index f597f7e8e8..f20778f048 100644 --- a/tests/test_plotting.py +++ b/tests/test_plotting.py @@ -30,7 +30,9 @@ from conftest import has_dependency, requires if not has_dependency("pyvista"): - pytest.skip(allow_module_level=True) + pytest.skip( + allow_module_level=True, reason="Skipping because 'pyvista' is not installed" + ) from ansys.mapdl.core.errors import ComponentDoesNotExits, MapdlRuntimeError from ansys.mapdl.core.plotting import GraphicsBackend diff --git a/tests/test_pool.py b/tests/test_pool.py index 7f8b5a1042..7799cf3e55 100644 --- a/tests/test_pool.py +++ b/tests/test_pool.py @@ -51,7 +51,9 @@ # skipping if ON_STUDENT and ON_LOCAL because we cannot spawn that many instances. if ON_STUDENT: - pytest.skip(allow_module_level=True) + pytest.skip( + allow_module_level=True, reason="Skipping Pool tests on student version." + ) skip_if_ignore_pool = pytest.mark.skipif( diff --git a/tests/test_theme.py b/tests/test_theme.py index 25073c34ee..06b646d91a 100644 --- a/tests/test_theme.py +++ b/tests/test_theme.py @@ -25,7 +25,9 @@ from conftest import has_dependency if not has_dependency("pyvista"): - pytest.skip(allow_module_level=True) + pytest.skip( + allow_module_level=True, reason="Skipping because 'pyvista' is not installed" + ) import matplotlib import numpy as np