gh-36024: `sage -fixdoctests`: Handle directory names, call `sage -t` only once
    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes #1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

<!-- Why is this change required? What problem does it solve? -->
We change the handling of file name arguments in `sage -fixdoctests`:
Instead of calling `sage -t` on files one by one, we just pass them to
`sage -t` as a whole and then handle its output for all files. This is
much faster because of the startup overhead of `sage -t`, and also adds
the handling of directory names.
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes #12345". -->
Cherry-picked from
- #35095
Part of
- #29705
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on
- #12345: short description why this is a dependency
- #34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: #36024
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee, Matthias Köppe

Author: Release Manager

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
@@ -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:
@@ -224,6 +228,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:
@@ -305,17 +311,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:
+            return args.filename[1]
+        else:
+            return None
+        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 ''
@@ -329,38 +339,61 @@ 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
+    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.")
 
-for input, output in zip(inputs, outputs):
-    if (skipfile_result := skipfile(input, True, log=print)) is True:
-        continue
-
-    if args.no_test:
-        doc_out = ''
+        input_filenames = [args.filename[0]]
     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)}')
+        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)}')
 
-        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', warn_long=10000)
+    DC = DocTestController(DD, input_filenames)
+    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
@@ -392,20 +425,26 @@ for input, output in zip(inputs, outputs):
                 persistent_optional_tags = {}
 
         if src_in_lines != shallow_copy_of_src_in_lines:
-            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}'")
+
+process_grouped_blocks(
+    itertools.chain(itertools.groupby(doctests, block_filename),
+                    ((filename, []) for filename in expanded_filename_args())))

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")