From 50baf64b75369ccac66a1340ea593d11f14a772e Mon Sep 17 00:00:00 2001 From: Matthias Koeppe Date: Wed, 12 Jul 2023 12:49:39 -0700 Subject: [PATCH 1/6] src/bin/sage-fixdoctests: Do not act on 'NameError: x' --- src/bin/sage-fixdoctests | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/bin/sage-fixdoctests b/src/bin/sage-fixdoctests index 0c06d448887..a644c6e4d10 100755 --- a/src/bin/sage-fixdoctests +++ b/src/bin/sage-fixdoctests @@ -224,6 +224,8 @@ def process_block(block, src_in_lines, file_optional_tags): # NameError from top level, so keep it brief if m := re.match("NameError: name '(.*)'", got[index_NameError:]): name = m.group(1) + if name == 'x': # Don't mark it '# needs sage.symbolic'; that's almost always wrong + return if feature := name_feature(name): add_tags = [feature.name] else: From 7302ff3ac8834961cc703d7e48770c053d846438 Mon Sep 17 00:00:00 2001 From: Matthias Koeppe Date: Wed, 12 Jul 2023 16:09:02 -0700 Subject: [PATCH 2/6] src/bin/sage-fixdoctests: Run doctester for all files in parallel, handle directories --- src/bin/sage-fixdoctests | 85 ++++++++++++++++++++++++++-------------- 1 file changed, 55 insertions(+), 30 deletions(-) diff --git a/src/bin/sage-fixdoctests b/src/bin/sage-fixdoctests index a644c6e4d10..074eb280347 100755 --- a/src/bin/sage-fixdoctests +++ b/src/bin/sage-fixdoctests @@ -40,7 +40,7 @@ import sys from argparse import ArgumentParser, FileType from pathlib import Path -from sage.doctest.control import skipfile +from sage.doctest.control import DocTestDefaults, DocTestController from sage.doctest.parsing import parse_file_optional_tags, parse_optional_tags, unparse_optional_tags, update_optional_tags from sage.env import SAGE_ROOT from sage.features import PythonModule @@ -307,17 +307,21 @@ def process_block(block, src_in_lines, file_optional_tags): # set input and output files -if len(args.filename) == 2 and not args.overwrite and not args.no_overwrite: - inputs, outputs = [args.filename[0]], [args.filename[1]] - print("sage-fixdoctests: When passing two filenames, the second one is taken as an output filename; " - "this is deprecated. To pass two input filenames, use the option --overwrite.") -elif args.no_overwrite: - inputs, outputs = args.filename, [input + ".fixed" for input in args.filename] -else: - inputs = outputs = args.filename +def output_filename(filename): + if len(args.filename) == 2 and not args.overwrite and not args.no_overwrite: + if args.filename[0] == filename: + print("sage-fixdoctests: When passing two filenames, the second one is taken as an output filename; " + "this is deprecated. To pass two input filenames, use the option --overwrite.") + return args.filename[1] + return filename + ".fixed" + if args.no_overwrite: + return filename + ".fixed" + return filename # Test the doctester, putting the output of the test into sage's temporary directory -if not args.no_test: +if args.no_test: + doc_out = '' +else: executable = f'{os.path.relpath(args.venv)}/bin/sage' if args.venv else 'sage' environment_args = f'--environment {args.environment} ' if args.environment != runtest_default_environment else '' long_args = f'--long ' if args.long else '' @@ -331,38 +335,54 @@ if not args.no_test: if status := os.waitstatus_to_exitcode(os.system(f'{cmdline} > {shlex.quote(doc_file)}')): print(f'Doctester exited with error status {status}') sys.exit(status) + # Run the doctester, putting the output of the test into sage's temporary directory + input_args = " ".join(shlex.quote(f) for f in args.filename) + cmdline = f'{shlex.quote(executable)} -t -p {environment_args}{long_args}{probe_args}{lib_args}{input_args}' + print(f'Running "{cmdline}"') + os.system(f'{cmdline} > {shlex.quote(doc_file)}') -for input, output in zip(inputs, outputs): - if (skipfile_result := skipfile(input, True, log=print)) is True: - continue - - if args.no_test: - doc_out = '' - else: - # Run the doctester, putting the output of the test into sage's temporary directory - cmdline = f'{shlex.quote(executable)} -t {environment_args}{long_args}{probe_args}{lib_args}{shlex.quote(input)}' - print(f'Running "{cmdline}"') - os.system(f'{cmdline} > {shlex.quote(doc_file)}') - - with open(doc_file, 'r') as doc: - doc_out = doc.read() + with open(doc_file, 'r') as doc: + doc_out = doc.read() # echo control messages for m in re.finditer('^Skipping .*', doc_out, re.MULTILINE): print('sage-runtests: ' + m.group(0)) - break - else: - sep = "**********************************************************************\n" - doctests = doc_out.split(sep) + +sep = "**********************************************************************\n" +doctests = doc_out.split(sep) + +seen = set() + +def block_filename(block): + if not (m := re.match('File "([^"]*)", line ([0-9]+), in ', block)): + return None + return m.group(1) + +def expanded_filename_args(): + DD = DocTestDefaults(optional='all') + DC = DocTestController(DD, args.filename) + DC.add_files() + DC.expand_files_into_sources() + for source in DC.sources: + yield source.path + +def process_grouped_blocks(grouped_iterator): + + for input, blocks in grouped_iterator: + + if not input: # Blocks of noise + continue + if input in seen: + continue + seen.add(input) with open(input, 'r') as test_file: src_in = test_file.read() src_in_lines = src_in.splitlines() shallow_copy_of_src_in_lines = list(src_in_lines) - file_optional_tags = set(parse_file_optional_tags(enumerate(src_in_lines))) - for block in doctests: + for block in blocks: process_block(block, src_in_lines, file_optional_tags) # Now source line numbers do not matter any more, and lines can be real lines again @@ -394,6 +414,7 @@ for input, output in zip(inputs, outputs): persistent_optional_tags = {} if src_in_lines != shallow_copy_of_src_in_lines: + output = output_filename(input) with open(output, 'w') as test_output: for line in src_in_lines: if line is None: @@ -411,3 +432,7 @@ for input, output in zip(inputs, outputs): subprocess.call(['git', '--no-pager', 'diff', relative], cwd=SAGE_ROOT) else: print(f"No fixes made in '{input}'") + +process_grouped_blocks( + itertools.chain(itertools.groupby(doctests, block_filename), + ((filename, []) for filename in expanded_filename_args()))) From 255e2d82ac247ecb6cb0642397c7d23cd62bca88 Mon Sep 17 00:00:00 2001 From: Matthias Koeppe Date: Sun, 16 Jul 2023 21:08:16 -0700 Subject: [PATCH 3/6] sage -fixdoctests: Handle ImportError too --- src/bin/sage-fixdoctests | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/bin/sage-fixdoctests b/src/bin/sage-fixdoctests index 074eb280347..3a0b904b8b3 100755 --- a/src/bin/sage-fixdoctests +++ b/src/bin/sage-fixdoctests @@ -189,8 +189,12 @@ def process_block(block, src_in_lines, file_optional_tags): return # Error testing. - if m := re.search(r"ModuleNotFoundError: No module named '([^']*)'", block): - module = m.group(1) + if m := re.search(r"(?:ModuleNotFoundError: No module named|ImportError: cannot import name '([^']*)' from) '([^']*)'", block): + if m.group(1): + # "ImportError: cannot import name 'function_field_polymod' from 'sage.rings.function_field' (unknown location)" + module = m.group(2) + '.' + m.group(1) + else: + module = m.group(2) asked_why = re.search('#.*(why|explain)', src_in_lines[first_line_num - 1]) optional = module_feature(module) if optional and optional.name not in file_optional_tags: From f631de2af8b58859bab72d116198eaae8e951d17 Mon Sep 17 00:00:00 2001 From: Matthias Koeppe Date: Sat, 15 Jul 2023 16:07:49 -0700 Subject: [PATCH 4/6] sage -t --if-installed: Ignore all files except .py and .pyx --- src/sage/doctest/control.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/sage/doctest/control.py b/src/sage/doctest/control.py index 4844662073d..9261c69dd4b 100644 --- a/src/sage/doctest/control.py +++ b/src/sage/doctest/control.py @@ -281,6 +281,10 @@ def skipfile(filename, tested_optional_tags=False, *, if log: log(f"Skipping '{filename}' because it does not have one of the recognized file name extensions") return True + if if_installed and ext not in ('.py', '.pyx'): + if log: + log(f"Skipping '{filename}' because it is not the source file of a Python module") + return True if "jupyter_execute" in filename: if log: log(f"Skipping '{filename}' because it is created by the jupyter-sphinx extension for internal use and should not be tested") From fe792827001e37b8d43e952bdbdd8ca6da334c8f Mon Sep 17 00:00:00 2001 From: Matthias Koeppe Date: Wed, 2 Aug 2023 19:52:49 -0700 Subject: [PATCH 5/6] sage -fixdoctests: Fix handling of 2 file arguments --- src/bin/sage-fixdoctests | 46 +++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/src/bin/sage-fixdoctests b/src/bin/sage-fixdoctests index 3a0b904b8b3..1b99e0e38ea 100755 --- a/src/bin/sage-fixdoctests +++ b/src/bin/sage-fixdoctests @@ -314,9 +314,9 @@ def process_block(block, src_in_lines, file_optional_tags): def output_filename(filename): if len(args.filename) == 2 and not args.overwrite and not args.no_overwrite: if args.filename[0] == filename: - print("sage-fixdoctests: When passing two filenames, the second one is taken as an output filename; " - "this is deprecated. To pass two input filenames, use the option --overwrite.") return args.filename[1] + else: + return None return filename + ".fixed" if args.no_overwrite: return filename + ".fixed" @@ -340,7 +340,14 @@ else: print(f'Doctester exited with error status {status}') sys.exit(status) # Run the doctester, putting the output of the test into sage's temporary directory - input_args = " ".join(shlex.quote(f) for f in args.filename) + if len(args.filename) == 2 and not args.overwrite and not args.no_overwrite: + print("sage-fixdoctests: When passing two filenames, the second one is taken as an output filename; " + "this is deprecated. To pass two input filenames, use the option --overwrite.") + + input_filenames = [args.filename[0]] + else: + input_filenames = args.filename + input_args = " ".join(shlex.quote(f) for f in input_filenames) cmdline = f'{shlex.quote(executable)} -t -p {environment_args}{long_args}{probe_args}{lib_args}{input_args}' print(f'Running "{cmdline}"') os.system(f'{cmdline} > {shlex.quote(doc_file)}') @@ -364,7 +371,7 @@ def block_filename(block): def expanded_filename_args(): DD = DocTestDefaults(optional='all') - DC = DocTestController(DD, args.filename) + DC = DocTestController(DD, input_filenames) DC.add_files() DC.expand_files_into_sources() for source in DC.sources: @@ -418,22 +425,23 @@ def process_grouped_blocks(grouped_iterator): persistent_optional_tags = {} if src_in_lines != shallow_copy_of_src_in_lines: - output = output_filename(input) - with open(output, 'w') as test_output: - for line in src_in_lines: - if line is None: - continue - test_output.write(line) - test_output.write('\n') - - # Show summary of changes - if input != output: - print("The fixed doctests have been saved as '{0}'.".format(output)) + if (output := output_filename(input)) is None: + print(f"Not saving modifications made in '{input}'") else: - relative = os.path.relpath(output, SAGE_ROOT) - print(f"The input file '{output}' has been overwritten.") - if not relative.startswith('..'): - subprocess.call(['git', '--no-pager', 'diff', relative], cwd=SAGE_ROOT) + with open(output, 'w') as test_output: + for line in src_in_lines: + if line is None: + continue + test_output.write(line) + test_output.write('\n') + # Show summary of changes + if input != output : + print("The fixed doctests have been saved as '{0}'.".format(output)) + else: + relative = os.path.relpath(output, SAGE_ROOT) + print(f"The input file '{output}' has been overwritten.") + if not relative.startswith('..'): + subprocess.call(['git', '--no-pager', 'diff', relative], cwd=SAGE_ROOT) else: print(f"No fixes made in '{input}'") From 2dccf184dd769b9dfc7cb0afe76082381122f87b Mon Sep 17 00:00:00 2001 From: Matthias Koeppe Date: Wed, 2 Aug 2023 22:51:42 -0700 Subject: [PATCH 6/6] sage -fixdoctests: Suppress 'too few successful tests' message --- src/bin/sage-fixdoctests | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bin/sage-fixdoctests b/src/bin/sage-fixdoctests index 1b99e0e38ea..50387881ad3 100755 --- a/src/bin/sage-fixdoctests +++ b/src/bin/sage-fixdoctests @@ -370,7 +370,7 @@ def block_filename(block): return m.group(1) def expanded_filename_args(): - DD = DocTestDefaults(optional='all') + DD = DocTestDefaults(optional='all', warn_long=10000) DC = DocTestController(DD, input_filenames) DC.add_files() DC.expand_files_into_sources()