Skip to content

Commit

Permalink
gh-36938: CI Build & Test: Show doctest failures, warnings as annotat…
Browse files Browse the repository at this point in the history
…ions in the 'Files changed' tab

    
<!-- ^^^^^
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 -->

This prints doctest failures as github annotations.

For illustrating this new feature, the branch on #36558 provokes some
errors.

As seen in the "Files changed" tab over there
(https://github.com/sagemath/sage/pull/36558/files):
- Doctest failures show as errors next to the doctest in source code
- Doctest warnings show as "warnings"
- Doctest failures from files that "failed in baseline" show as
"notices"

These messages can also be seen in the Summary page of the workflow
(https://github.com/sagemath/sage/actions/runs/7359388426?pr=36558).
Clicking on a message leads to the source code location.

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes #12345". -->
<!-- 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.
- [ ] 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: ...
-->
- Depends on #36936

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: #36938
Reported by: Matthias Köppe
Reviewer(s): Alex J Best, Matthias Köppe, Tobias Diez
  • Loading branch information
Release Manager committed Feb 1, 2024
2 parents caf7cf8 + 0c52169 commit eb0fc15
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 13 deletions.
10 changes: 5 additions & 5 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -197,15 +197,15 @@ jobs:
- name: Test all files (sage -t --all --long)
if: (success() || failure()) && steps.build.outcome == 'success'
run: |
../sage -python -m pip install coverage
../sage -python -m coverage run ./bin/sage-runtests --all --long -p2 --random-seed=286735480429121101562228604801325644303
working-directory: ./worktree-image/src
./sage -python -m pip install coverage
./sage -python -m coverage run --rcfile=src/tox.ini src/bin/sage-runtests --all --long -p2 --format github --random-seed=286735480429121101562228604801325644303
working-directory: ./worktree-image

- name: Prepare coverage results
if: (success() || failure()) && steps.build.outcome == 'success'
run: |
./venv/bin/python3 -m coverage combine src/.coverage/
./venv/bin/python3 -m coverage xml
./sage -python -m coverage combine --rcfile=src/tox.ini
./sage -python -m coverage xml --rcfile=src/tox.ini
mkdir -p coverage-report
mv coverage.xml coverage-report/
working-directory: ./worktree-image
Expand Down
2 changes: 2 additions & 0 deletions src/bin/sage-runtests
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ if __name__ == "__main__":
what.add_argument("--installed", action="store_true", default=False, help="test all installed modules of the Sage library")
parser.add_argument("--logfile", type=argparse.FileType('a'), metavar="FILE", help="log all output to FILE")

parser.add_argument("--format", choices=["sage", "github"], default="sage",
help="set format of error messages and warnings")
parser.add_argument("-l", "--long", action="store_true", default=False, help="include lines with the phrase 'long time'")
parser.add_argument("-s", "--short", dest="target_walltime", nargs='?',
type=int, default=-1, const=300, metavar="SECONDS",
Expand Down
1 change: 1 addition & 0 deletions src/sage/doctest/control.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ def __init__(self, **kwds):
self.show_skipped = False
self.target_walltime = -1
self.baseline_stats_path = None
self.format = "sage"

# sage-runtests contains more optional tags. Technically, adding
# auto_optional_tags here is redundant, since that is added
Expand Down
60 changes: 52 additions & 8 deletions src/sage/doctest/forker.py
Original file line number Diff line number Diff line change
Expand Up @@ -1235,16 +1235,57 @@ def _failure_header(self, test, example, message='Failed example:'):
"""
out = [self.DIVIDER]
with OriginalSource(example):
if test.filename:
if test.lineno is not None and example.lineno is not None:
lineno = test.lineno + example.lineno + 1
if self.options.format == 'sage':
if test.filename:
if test.lineno is not None and example.lineno is not None:
lineno = test.lineno + example.lineno + 1
else:
lineno = '?'
out.append('File "%s", line %s, in %s' %
(test.filename, lineno, test.name))
else:
out.append('Line %s, in %s' % (example.lineno + 1, test.name))
out.append(message)
elif self.options.format == 'github':
# https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#using-workflow-commands-to-access-toolkit-functions
if message.startswith('Warning: '):
command = f'::warning title={message}'
message = message[len('Warning: '):]
elif self.baseline.get('failed', False):
command = f'::notice title={message}'
message += ' [failed in baseline]'
else:
lineno = '?'
out.append('File "%s", line %s, in %s' %
(test.filename, lineno, test.name))
command = f'::error title={message}'
if extra := getattr(example, 'extra', None):
message += f': {extra}'
if test.filename:
command += f',file={test.filename}'
if test.lineno is not None and example.lineno is not None:
lineno = test.lineno + example.lineno + 1
command += f',line={lineno}'
lineno = None
else:
command += f',line={example.lineno + 1}'
#
# Urlencoding trick for multi-line annotations
# https://github.com/actions/starter-workflows/issues/68#issuecomment-581479448
#
# This only affects the display in the workflow Summary, after clicking "Show more";
# the message needs to be long enough so that "Show more" becomes available.
# https://github.com/actions/toolkit/issues/193#issuecomment-1867084340
#
# Unfortunately, this trick does not make the annotations in the diff view multi-line.
#
if '\n' in message:
message = message.replace('\n', '%0A')
# The actual threshold for "Show more" to appear depends on the window size.
show_more_threshold = 500
if (pad := show_more_threshold - len(message)) > 0:
message += ' ' * pad
command += f'::{message}'
out.append(command)
else:
out.append('Line %s, in %s' % (example.lineno + 1, test.name))
out.append(message)
raise ValueError(f'unknown format option: {self.options.format}')
source = example.source
out.append(doctest._indent(source))
return '\n'.join(out)
Expand Down Expand Up @@ -1417,6 +1458,7 @@ def report_failure(self, out, test, example, got, globs):
"""
if not self.options.initial or self.no_failure_yet:
self.no_failure_yet = False
example.extra = f'Got: {got}'
returnval = doctest.DocTestRunner.report_failure(self, out, test, example, got)
if self.options.debug:
self._fakeout.stop_spoofing()
Expand Down Expand Up @@ -1569,6 +1611,8 @@ def report_unexpected_exception(self, out, test, example, exc_info):
"""
if not self.options.initial or self.no_failure_yet:
self.no_failure_yet = False

example.extra = "Exception raised:\n" + "".join(traceback.format_exception(*exc_info))
returnval = doctest.DocTestRunner.report_unexpected_exception(self, out, test, example, exc_info)
if self.options.debug:
self._fakeout.stop_spoofing()
Expand Down
4 changes: 4 additions & 0 deletions src/tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -299,3 +299,7 @@ source = sage
concurrency = multiprocessing
data_file = .coverage/.coverage
disable_warnings = no-data-collected
[coverage:report]
ignore_errors = True
skip_empty = True

0 comments on commit eb0fc15

Please sign in to comment.