From bec22a6c404fb791230f2338d47dc42091d4fa1f Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 25 Nov 2022 13:24:26 +0100 Subject: [PATCH 1/7] Fix counting of failures in PyTorch tests --- easybuild/easyblocks/p/pytorch.py | 73 ++++++++++++++++--------------- 1 file changed, 38 insertions(+), 35 deletions(-) diff --git a/easybuild/easyblocks/p/pytorch.py b/easybuild/easyblocks/p/pytorch.py index e72a701351..32a295c02c 100644 --- a/easybuild/easyblocks/p/pytorch.py +++ b/easybuild/easyblocks/p/pytorch.py @@ -273,23 +273,12 @@ def test_step(self): for hit in ran_tests_hits: test_cnt += int(hit) - # Get matches to create clear summary report, greps for patterns like: - # FAILED (errors=10, skipped=190, expected failures=6) - # test_fx failed! - regex = r"^Ran (?P[0-9]+) tests.*$\n\nFAILED \((?P.*)\)$\n(?:^(?:(?!failed!).)*$\n)*(?P.*) failed!$" # noqa: E501 - summary_matches = re.findall(regex, tests_out, re.M) - - # Get matches to create clear summary report, greps for patterns like: - # ===================== 2 failed, 128 passed, 2 skipped, 2 warnings in 3.43s ===================== - regex = r"^=+ (?P.*) in [0-9]+\.*[0-9]*[a-zA-Z]* =+$\n(?P.*) failed!$" - summary_matches_pattern2 = re.findall(regex, tests_out, re.M) - - # Count failures and errors def get_count_for_pattern(regex, text): - match = re.findall(regex, text, re.M) - if len(match) == 0: - return 0 - elif len(match) == 1: + """Match the regexp containing a single group and return the integer value of the matched group. + Return zero if no or more than 1 match was found and warn for the latter case + """ + match = re.findall(regex, text) + if len(match) == 1: return int(match[0]) elif len(match) > 1: # Shouldn't happen, but means something went wrong with the regular expressions. @@ -297,22 +286,41 @@ def get_count_for_pattern(regex, text): warn_msg = "Error in counting the number of test failures in the output of the PyTorch test suite.\n" warn_msg += "Please check the EasyBuild log to verify the number of failures (if any) was acceptable." print_warning(warn_msg) + return 0 + # Create clear summary report + failure_report = "" failure_cnt = 0 error_cnt = 0 - # Loop over first pattern to count failures/errors: - for summary in summary_matches: - failures = get_count_for_pattern(r"^.*(?[0-9]+).*$", summary[1]) - failure_cnt += failures - errs = get_count_for_pattern(r"^.*errors=(?P[0-9]+).*$", summary[1]) - error_cnt += errs - - # Loop over the second pattern to count failures/errors - for summary in summary_matches_pattern2: - failures = get_count_for_pattern(r"^.*[^0-9](?P[0-9]+) failed.*$", summary[0]) - failure_cnt += failures - errs = get_count_for_pattern(r"^.*[^0-9](?P[0-9]+) error.*$", summary[0]) - error_cnt += errs + + # Grep for patterns like: + # Ran 219 tests in 67.325s + # + # FAILED (errors=10, skipped=190, expected failures=6) + # test_fx failed! + regex = r"^Ran (?P[0-9]+) tests.*$\n\nFAILED \((?P.*)\)$\n(?:^(?:(?!failed!).)*$\n)*(?P.*) failed!$" # noqa: E501 + + for summary in re.findall(regex, tests_out, re.M): + # E.g. 'failures=3, errors=10, skipped=190, expected failures=6' + failure_summary = summary[1] + failure_report += "{test_suite} ({total} total tests, {failure_summary})\n".format( + test_suite=summary[2], total=summary[0], failure_summary=failure_summary + ) + failure_cnt += get_count_for_pattern(r"(?.*) in [0-9]+\.*[0-9]*[a-zA-Z]* =+$\n(?P.*) failed!$" + + for summary in re.findall(regex, tests_out, re.M): + # E.g. '2 failed, 128 passed, 2 skipped, 2 warnings' + failure_summary = summary[0] + failure_report += "{test_suite} ({failure_summary})\n".format( + test_suite=summary[1], failure_summary=failure_summary + ) + failure_cnt = get_count_for_pattern(r"([0-9]+) failed", failure_summary) + error_cnt = get_count_for_pattern(r"([0-9]+) error", failure_summary) # Calculate total number of unsuccesful tests failed_test_cnt = failure_cnt + error_cnt @@ -325,12 +333,7 @@ def get_count_for_pattern(regex, text): msg = "%d test %s, %d test %s (out of %d):\n" % ( failure_cnt, failure_or_failures, error_cnt, error_or_errors, test_cnt ) - for summary in summary_matches_pattern2: - msg += "{test_suite} ({failure_summary})\n".format(test_suite=summary[1], failure_summary=summary[0]) - for summary in summary_matches: - msg += "{test_suite} ({total} total tests, {failure_summary})\n".format( - test_suite=summary[2], total=summary[0], failure_summary=summary[1] - ) + msg += failure_summary if max_failed_tests == 0: raise EasyBuildError(msg) From 4e8a97e62ea0be231665df76454a256c61d50534 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 25 Nov 2022 13:32:51 +0100 Subject: [PATCH 2/7] Also handle tests terminating by a signal --- easybuild/easyblocks/p/pytorch.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/easybuild/easyblocks/p/pytorch.py b/easybuild/easyblocks/p/pytorch.py index 32a295c02c..b1c4577404 100644 --- a/easybuild/easyblocks/p/pytorch.py +++ b/easybuild/easyblocks/p/pytorch.py @@ -298,7 +298,10 @@ def get_count_for_pattern(regex, text): # # FAILED (errors=10, skipped=190, expected failures=6) # test_fx failed! - regex = r"^Ran (?P[0-9]+) tests.*$\n\nFAILED \((?P.*)\)$\n(?:^(?:(?!failed!).)*$\n)*(?P.*) failed!$" # noqa: E501 + regex = (r"^Ran (?P[0-9]+) tests.*$\n\n" + r"FAILED \((?P.*)\)$\n" + r"(?:^(?:(?!failed!).)*$\n)*" + r"(?P.*) failed!(?: Received signal: \w+)?\s*$") for summary in re.findall(regex, tests_out, re.M): # E.g. 'failures=3, errors=10, skipped=190, expected failures=6' From b0e63fea20468befdbfac146cf07d3b1993def36 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Sat, 26 Nov 2022 11:19:48 +0100 Subject: [PATCH 3/7] Add (not overwrite) failure/error counts Co-authored-by: Simon Branford <4967+branfosj@users.noreply.github.com> --- easybuild/easyblocks/p/pytorch.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/easybuild/easyblocks/p/pytorch.py b/easybuild/easyblocks/p/pytorch.py index b1c4577404..fa28329351 100644 --- a/easybuild/easyblocks/p/pytorch.py +++ b/easybuild/easyblocks/p/pytorch.py @@ -322,8 +322,8 @@ def get_count_for_pattern(regex, text): failure_report += "{test_suite} ({failure_summary})\n".format( test_suite=summary[1], failure_summary=failure_summary ) - failure_cnt = get_count_for_pattern(r"([0-9]+) failed", failure_summary) - error_cnt = get_count_for_pattern(r"([0-9]+) error", failure_summary) + failure_cnt += get_count_for_pattern(r"([0-9]+) failed", failure_summary) + error_cnt += get_count_for_pattern(r"([0-9]+) error", failure_summary) # Calculate total number of unsuccesful tests failed_test_cnt = failure_cnt + error_cnt From 3398051e0022851231c1fcd675ade7a694b1beca Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 28 Nov 2022 10:52:43 +0100 Subject: [PATCH 4/7] Fix failure_report and use finditer --- easybuild/easyblocks/p/pytorch.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/easybuild/easyblocks/p/pytorch.py b/easybuild/easyblocks/p/pytorch.py index fa28329351..f991d36aa8 100644 --- a/easybuild/easyblocks/p/pytorch.py +++ b/easybuild/easyblocks/p/pytorch.py @@ -303,11 +303,12 @@ def get_count_for_pattern(regex, text): r"(?:^(?:(?!failed!).)*$\n)*" r"(?P.*) failed!(?: Received signal: \w+)?\s*$") - for summary in re.findall(regex, tests_out, re.M): + for m in re.finditer(regex, tests_out, re.M): # E.g. 'failures=3, errors=10, skipped=190, expected failures=6' - failure_summary = summary[1] + failure_summary = m['failure_summary'] + total, test_suite = m.group('test_cnt', 'failed_test_suite_name') failure_report += "{test_suite} ({total} total tests, {failure_summary})\n".format( - test_suite=summary[2], total=summary[0], failure_summary=failure_summary + test_suite=test_suite, total=total, failure_summary=failure_summary ) failure_cnt += get_count_for_pattern(r"(?.*) in [0-9]+\.*[0-9]*[a-zA-Z]* =+$\n(?P.*) failed!$" - for summary in re.findall(regex, tests_out, re.M): + for m in re.finditer(regex, tests_out, re.M): # E.g. '2 failed, 128 passed, 2 skipped, 2 warnings' - failure_summary = summary[0] + failure_summary = m['failure_summary'] + test_suite = m['failed_test_suite_name'] failure_report += "{test_suite} ({failure_summary})\n".format( - test_suite=summary[1], failure_summary=failure_summary + test_suite=test_suite, failure_summary=failure_summary ) failure_cnt += get_count_for_pattern(r"([0-9]+) failed", failure_summary) error_cnt += get_count_for_pattern(r"([0-9]+) error", failure_summary) @@ -336,7 +338,7 @@ def get_count_for_pattern(regex, text): msg = "%d test %s, %d test %s (out of %d):\n" % ( failure_cnt, failure_or_failures, error_cnt, error_or_errors, test_cnt ) - msg += failure_summary + msg += failure_report if max_failed_tests == 0: raise EasyBuildError(msg) From 55bdf743cd811330ff68c626737a675473581fe3 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 28 Nov 2022 12:08:28 +0100 Subject: [PATCH 5/7] Include also failed tests due to syntax errors etc. --- easybuild/easyblocks/p/pytorch.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/easybuild/easyblocks/p/pytorch.py b/easybuild/easyblocks/p/pytorch.py index f991d36aa8..40f4164b46 100644 --- a/easybuild/easyblocks/p/pytorch.py +++ b/easybuild/easyblocks/p/pytorch.py @@ -40,6 +40,7 @@ from easybuild.tools.filetools import apply_regex_substitutions, mkdir, symlink from easybuild.tools.modules import get_software_root, get_software_version from easybuild.tools.systemtools import POWER, get_cpu_architecture +from easybuild.tools.utilities import nub class EB_PyTorch(PythonPackage): @@ -292,6 +293,7 @@ def get_count_for_pattern(regex, text): failure_report = "" failure_cnt = 0 error_cnt = 0 + failed_test_suites = [] # Grep for patterns like: # Ran 219 tests in 67.325s @@ -312,6 +314,7 @@ def get_count_for_pattern(regex, text): ) failure_cnt += get_count_for_pattern(r"(?.*) failed!(?: Received signal: \w+)?\s*$", tests_out, re.M) + ) + if failed_test_suites_2 != failed_test_suites: + failure_report = '\n'.join('* %s' % t for t in sorted(failed_test_suites_2)) + failure_report if failed_test_cnt > 0: max_failed_tests = self.cfg['max_failed_tests'] @@ -340,7 +352,8 @@ def get_count_for_pattern(regex, text): ) msg += failure_report - if max_failed_tests == 0: + # If no tests are supposed to fail or some failed for which we were not able to count errors fail now + if max_failed_tests == 0 or failed_test_suites_2 != failed_test_suites: raise EasyBuildError(msg) else: msg += '\n\n' + ' '.join([ @@ -360,6 +373,8 @@ def get_count_for_pattern(regex, text): if failed_test_cnt > max_failed_tests: raise EasyBuildError("Too many failed tests (%d), maximum allowed is %d", failed_test_cnt, max_failed_tests) + elif failure_report: + raise EasyBuildError("Test command had non-zero exit code (%s)!\n%s", tests_ec, failure_report) elif tests_ec: raise EasyBuildError("Test command had non-zero exit code (%s), but no failed tests found?!", tests_ec) From c6daaef924bcbe87d42d033f8babf35ffade9f21 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 28 Nov 2022 15:48:52 +0100 Subject: [PATCH 6/7] Improve error message --- easybuild/easyblocks/p/pytorch.py | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/easybuild/easyblocks/p/pytorch.py b/easybuild/easyblocks/p/pytorch.py index 40f4164b46..52e1b52e98 100644 --- a/easybuild/easyblocks/p/pytorch.py +++ b/easybuild/easyblocks/p/pytorch.py @@ -267,12 +267,7 @@ def test_step(self): 'excluded_tests': ' '.join(excluded_tests) }) - (tests_out, tests_ec) = super(EB_PyTorch, self).test_step(return_output_ec=True) - - ran_tests_hits = re.findall(r"^Ran (?P[0-9]+) tests in", tests_out, re.M) - test_cnt = 0 - for hit in ran_tests_hits: - test_cnt += int(hit) + tests_out, tests_ec = super(EB_PyTorch, self).test_step(return_output_ec=True) def get_count_for_pattern(regex, text): """Match the regexp containing a single group and return the integer value of the matched group. @@ -331,29 +326,36 @@ def get_count_for_pattern(regex, text): error_cnt += get_count_for_pattern(r"([0-9]+) error", failure_summary) failed_test_suites.append(test_suite) - # Calculate total number of unsuccesful tests - failed_test_cnt = failure_cnt + error_cnt # Make the names unique failed_test_suites = nub(failed_test_suites) # Gather all failed tests suites in case we missed any (e.g. when it exited due to syntax errors) - failed_test_suites_2 = nub( + all_failed_test_suites = nub( re.findall(r"^(?P.*) failed!(?: Received signal: \w+)?\s*$", tests_out, re.M) ) - if failed_test_suites_2 != failed_test_suites: - failure_report = '\n'.join('* %s' % t for t in sorted(failed_test_suites_2)) + failure_report + # If we missed any test suites prepend a list of all failed test suites + if failed_test_suites != all_failed_test_suites: + failure_report_save = failure_report + failure_report = 'Failed tests (suites/files):\n' + failure_report += '\n'.join('* %s' % t for t in sorted(all_failed_test_suites)) + if failure_report_save: + failure_report += '\n' + failure_report_save + + # Calculate total number of unsuccesful and total tests + failed_test_cnt = failure_cnt + error_cnt + test_cnt = sum(int(hit) for hit in re.findall(r"^Ran (?P[0-9]+) tests in", tests_out, re.M)) if failed_test_cnt > 0: max_failed_tests = self.cfg['max_failed_tests'] - failure_or_failures = 'failures' if failure_cnt > 1 else 'failure' - error_or_errors = 'errors' if error_cnt > 1 else 'error' + failure_or_failures = 'failure' if failure_cnt == 1 else 'failures' + error_or_errors = 'error' if error_cnt == 1 else 'errors' msg = "%d test %s, %d test %s (out of %d):\n" % ( failure_cnt, failure_or_failures, error_cnt, error_or_errors, test_cnt ) msg += failure_report # If no tests are supposed to fail or some failed for which we were not able to count errors fail now - if max_failed_tests == 0 or failed_test_suites_2 != failed_test_suites: + if max_failed_tests == 0 or failed_test_suites != all_failed_test_suites: raise EasyBuildError(msg) else: msg += '\n\n' + ' '.join([ From fa23f2af374a1891b1d9cd610e1d89e57c8a56ec Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Tue, 29 Nov 2022 10:14:09 +0100 Subject: [PATCH 7/7] Compare sorted list of test suites --- easybuild/easyblocks/p/pytorch.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/easybuild/easyblocks/p/pytorch.py b/easybuild/easyblocks/p/pytorch.py index 52e1b52e98..ecdbee711e 100644 --- a/easybuild/easyblocks/p/pytorch.py +++ b/easybuild/easyblocks/p/pytorch.py @@ -40,7 +40,6 @@ from easybuild.tools.filetools import apply_regex_substitutions, mkdir, symlink from easybuild.tools.modules import get_software_root, get_software_version from easybuild.tools.systemtools import POWER, get_cpu_architecture -from easybuild.tools.utilities import nub class EB_PyTorch(PythonPackage): @@ -326,17 +325,18 @@ def get_count_for_pattern(regex, text): error_cnt += get_count_for_pattern(r"([0-9]+) error", failure_summary) failed_test_suites.append(test_suite) - # Make the names unique - failed_test_suites = nub(failed_test_suites) + # Make the names unique and sorted + failed_test_suites = sorted(set(failed_test_suites)) # Gather all failed tests suites in case we missed any (e.g. when it exited due to syntax errors) - all_failed_test_suites = nub( + # Also unique and sorted to be able to compare the lists below + all_failed_test_suites = sorted(set( re.findall(r"^(?P.*) failed!(?: Received signal: \w+)?\s*$", tests_out, re.M) - ) + )) # If we missed any test suites prepend a list of all failed test suites if failed_test_suites != all_failed_test_suites: failure_report_save = failure_report failure_report = 'Failed tests (suites/files):\n' - failure_report += '\n'.join('* %s' % t for t in sorted(all_failed_test_suites)) + failure_report += '\n'.join('* %s' % t for t in all_failed_test_suites) if failure_report_save: failure_report += '\n' + failure_report_save