`src/sage/doctest/forker.py`: Show '# [failed in baseline]' earlier #36936

mkoeppe · 2023-12-20T21:39:05Z

The note "[failed in baseline]" already appears in the doctest failure summary.

Here we add the note also to the sage -t line during doctesting:

https://github.com/sagemath/sage/actions/runs/7281940815/job/19843412065?pr=36936#step:11:2744

sage -t --random-seed=256963700569517996050547927079750684585 src/sage/combinat/combinatorial_map.py
    [75 tests, 0.05 s]
sage -t --random-seed=256963700569517996050547927079750684585 src/sage/combinat/cluster_algebra_quiver/quiver.py  # [failed in baseline]
    [320 tests, 3.73 s]
sage -t --random-seed=256963700569517996050547927079750684585 src/sage/combinat/composition_signed.py
    [20 tests, 0.16 s]

https://github.com/sagemath/sage/actions/runs/7281940815/job/19843412065?pr=36936#step:11:9192

sage -t --random-seed=256963700569517996050547927079750684585 src/sage_setup/setenv.py
    [0 tests, 0.00 s]
sage -t --random-seed=256963700569517996050547927079750684585 src/sage_setup/clean.py  # [failed in baseline]
**********************************************************************
File "src/sage_setup/clean.py", line 104, in sage_setup.clean._find_stale_files
Failed example:
    for f in stale_iter:
        if f.endswith(skip_extensions): continue
        if '/ext_data/' in f: continue
        print('Found stale file: ' + f)
Expected nothing
Got:
    Found stale file: sage/tests/books/judson-abstract-algebra/homomorph-sage-exercises.py
    Found stale file: sage/tests/books/judson-abstract-algebra/actions-sage-exercises.py
    Found stale file: sage/tests/books/judson-abstract-algebra/homomorph-sage.py

The changes in the code that implement this are also preparation for:

sage -t --format github (illustration) #36558.

📝 Checklist

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

kwankyu · 2023-12-21T03:56:58Z

How about sage -t .... # fails in baseline?

The test is not yet run, so we should say that "sage -t ... is expected to fail but this failure is in the baseline". Hence no past tense.

And no brackets?

mkoeppe · 2023-12-21T04:06:20Z

Not sure about the wording. Would it help if we include the filename (.github/workflows/ci-conda-known-test-failures.json) in the messages somehow? This name is clearer than the very neutral "baseline".

kwankyu · 2023-12-21T04:30:31Z

Would it help if we include the filename (.github/workflows/ci-conda-known-test-failures.json) in the messages somehow? This name is clearer than the very neutral "baseline".

Better to be short. Anyway, developers should learn about the precise meaning of the message if they are interested. We cannot explain everything in the small space without cluttering.

kwankyu · 2023-12-21T04:34:12Z

May be "conda baseline"?

mkoeppe · 2023-12-21T05:16:34Z

Would it help if we include the filename (.github/workflows/ci-conda-known-test-failures.json) in the messages somehow?

OK, I'm logging it just once at the beginning

mkoeppe · 2023-12-21T05:17:49Z

The test is not yet run, so we should say that "sage -t ... is expected to fail but this failure is in the baseline". Hence no past tense.

In my view, the past tense refers to the time when the baseline was established

kwankyu · 2023-12-21T06:00:31Z

Then I don't understand the phrase "failed in the baseline" here.

By the dictionary definition, "the baseline" is the line from which we measure quality of someone or something.

We collected a list of files that fail persistently (or randomly persistently), and regard the list as part of "the baseline". So if the file fails again in this run, then it is not a failure measured from "the baseline". You express this by "Failed (but it is) in the baseline". No?

Now you use the same phrase in "sage -t ... # failed in the baseline". The comment is about "sage -t ...". At this moment, nothing yet failed or it may not fail at all. I cannot understand the same phrase "failed in the baseline" as above. You want to say that "this may fail but it is in the baseline". No?

kwankyu · 2023-12-21T06:10:18Z

In my view, the past tense refers to the time when the baseline was established

I see. So you mean "[This file] failed when the baseline was established" by "failed in the baseline".

mkoeppe · 2023-12-21T06:14:35Z

Yes. And in the summary line, when it says

sage -t --random-seed=271115389197827456996254478912680781530 src/ sage/doctest/sources.py  # 2 doctests failed [failed in baseline]

... it means it failed just now (and also it failed when the baseline was established).

kwankyu · 2023-12-21T06:15:58Z

OK. It makes sense to me now.

kwankyu · 2023-12-21T06:20:10Z

... it means it failed just now (and also it failed when the baseline was established).

You may want to use this explanation when you explain the new facility in the developer manual or in the release tour.

mkoeppe · 2023-12-21T06:21:34Z

it's already in the manual

mkoeppe · 2023-12-21T06:22:30Z

bottom of this subsection: https://doc.sagemath.org/html/en/developer/doctesting.html#auxiliary-files

kwankyu · 2023-12-21T06:49:31Z

Yes. The phrase "[failed in baseline]" is explained there. Good enough.

Here

https://doc.sagemath.org/html/en/developer/doctesting.html#:~:text=%2D%2Dbaseline%2Dstats%2Dpath%20known%2Dtest%2Dfailures.json

"=" is necessary.

Otherwise looks good to me. It works well.

mkoeppe · 2023-12-21T06:52:08Z

"=" is necessary.

Both ways of writing are actually fine with the Python argparse. See examples just above that do --stats-path ~/.sage/timings2.json or --logfile test1.log

kwankyu · 2023-12-21T06:52:51Z

Instead of timing2.json, .github/workflows/ci-conda-known-test-failures.json may be a better example.

There's no "failed": true in timing2.json.

kwankyu · 2023-12-21T06:54:04Z

"=" is necessary.

Both ways of writing are actually fine with the Python argparse. See examples just above that do --stats-path ~/.sage/timings2.json or --logfile test1.log

But

$ sage -t --baseline-stats-path .github/workflows/ci-conda-known-test-failures.json src/sage_setup/clean.py
usage: sage -t [options] filenames
sage-runtests: error: argument --baseline_stats_path/--baseline-stats-path: expected one argument

mkoeppe · 2023-12-21T06:56:51Z

hm... let me check

kwankyu · 2023-12-21T07:06:27Z

Instead of timing2.json, .github/workflows/ci-conda-known-test-failures.json may be a better example.

There's no "failed": true in timing2.json.

Yes, there is one:

    "sage.graphs.generators.families": {
        "failed": true,
        "ntests": 446,
        "walltime": 11.227901220321655
    },

mkoeppe · 2023-12-21T07:25:01Z

Likely this custom code https://github.com/sagemath/sage/blob/develop/src/bin/sage-runtests#L131 is to blame

mkoeppe · 2023-12-21T07:25:25Z

I'll take another look at this tomorrow.

mkoeppe · 2023-12-21T18:49:15Z

$ sage -t --baseline-stats-path .github/workflows/ci-conda-known-test-failures.json src/sage_setup/clean.py
usage: sage -t [options] filenames
sage-runtests: error: argument --baseline_stats_path/--baseline-stats-path: expected one argument

Fixed now

mkoeppe · 2023-12-26T16:24:22Z

How does the commit deal with this failure? Why this failure is related with this PR?

It's just a codepath in the doctester that so far has not been subject to the baseline mechanism. Various codepaths that are for internal failures of the doctesting framework are like this; but this particular codepath is triggered by doctests that test some timeout functionality.

The failure itself is already added to the known failures in https://github.com/sagemath/sage/pull/36937/files#diff-c6591fddad6cbbb6477ab5e8cc342b6c7b89b09f2707c7cc7aa818b52693448dR77

…' here

…mework' baseline-aware

kwankyu · 2023-12-27T02:03:44Z

src/sage/doctest/reporting.py

                    if hasattr(result_dict, 'tb'):
                        log(result_dict.tb)
                    if hasattr(result_dict, 'walltime'):
                        stats[basename] = {"failed": True, "walltime": wall, "ntests": ntests}
                    else:
                        stats[basename] = {"failed": True, "walltime": 1e6, "ntests": ntests}
-                    self.error_status |= 64
+                    if not baseline.get('failed', False):  # e.g. AlarmInterrupt in doctesting framework
+                        self.error_status |= 64


The comment is hard to understand. How about

# We may get here by e.g. AlarmInterrupt in doctesting framework if not baseline.get('failed', False): self.error_status |= 64

?

Thanks; improved in f61cb9a

Sorry for nitpicking. But one reason why the comment is hard to understand is that the comment is inside the if statement. The comment is about the whole if statement, I think.

We add a comment for the thing which is below or to the left. No? (Sometimes what I take as granted is not really granted...)

You are right, I've fixed it in cf9024f

kwankyu · 2023-12-27T02:04:59Z

Otherwise, lgtm.

github-actions · 2023-12-27T03:21:39Z

Documentation preview for this PR (built with commit cf9024f; changes) is ready! 🎉

kwankyu

Thanks.

mkoeppe · 2023-12-27T03:52:28Z

Thank you!

…seline]' earlier   The note "[failed in baseline]" already appears in the doctest failure summary. Here we add the note also to the `sage -t` line during doctesting: https://github.com/sagemath/sage/actions/runs/7281940815/job/19843412065 ?pr=36936#step:11:2744 ``` sage -t --random-seed=256963700569517996050547927079750684585 src/sage/combinat/combinatorial_map.py [75 tests, 0.05 s] sage -t --random-seed=256963700569517996050547927079750684585 src/sage/combinat/cluster_algebra_quiver/quiver.py # [failed in baseline] [320 tests, 3.73 s] sage -t --random-seed=256963700569517996050547927079750684585 src/sage/combinat/composition_signed.py [20 tests, 0.16 s] ``` https://github.com/sagemath/sage/actions/runs/7281940815/job/19843412065 ?pr=36936#step:11:9192 ``` sage -t --random-seed=256963700569517996050547927079750684585 src/sage_setup/setenv.py [0 tests, 0.00 s] sage -t --random-seed=256963700569517996050547927079750684585 src/sage_setup/clean.py # [failed in baseline] ********************************************************************** File "src/sage_setup/clean.py", line 104, in sage_setup.clean._find_stale_files Failed example: for f in stale_iter: if f.endswith(skip_extensions): continue if '/ext_data/' in f: continue print('Found stale file: ' + f) Expected nothing Got: Found stale file: sage/tests/books/judson-abstract- algebra/homomorph-sage-exercises.py Found stale file: sage/tests/books/judson-abstract-algebra/actions- sage-exercises.py Found stale file: sage/tests/books/judson-abstract- algebra/homomorph-sage.py ``` The changes in the code that implement this are also preparation for: - sagemath#36558.    ### 📝 Checklist     - [ ] 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   URL: sagemath#36936 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee, Matthias Köppe

Not all of the added failures are conda-specific, but the 6 instances of CI Conda amplify the error probability (sagemath#36694 (comment)) We also add info (ideally a link to the GitHub Issue where the failure is reported) to the failures and arrange for it to be printed by the doctester as part of the `[failed in baseline]` message. Marked as blocker so it takes effect in CI runs -- to reduce the noise from failures of the Conda CI that PR authors and reviewers find distracting.      ### 📝 Checklist     - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies  - Depends on sagemath#36936 (which refactors the `[failed in baseline]` printing)  URL: sagemath#36937 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee, Matthias Köppe, Tobias Diez

… annotations in the 'Files changed' tab   This prints doctest failures as github annotations. For illustrating this new feature, the branch on sagemath#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.    ### 📝 Checklist     - [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  - Depends on sagemath#36936  URL: sagemath#36938 Reported by: Matthias Köppe Reviewer(s): Alex J Best, Matthias Köppe, Tobias Diez

mkoeppe added p: minor / 4 c: doctest framework labels Dec 20, 2023

mkoeppe self-assigned this Dec 20, 2023

mkoeppe mentioned this pull request Dec 20, 2023

sage -t --format github (illustration) #36558

Closed

5 tasks

mkoeppe force-pushed the doctester-baselinestats-improvement branch from 044a721 to 48cde67 Compare December 20, 2023 23:11

mkoeppe added the s: needs review label Dec 21, 2023

mkoeppe mentioned this pull request Dec 21, 2023

CI Build & Test: Show doctest failures, warnings as annotations in the 'Files changed' tab #36938

Merged

5 tasks

mkoeppe requested a review from kwankyu December 21, 2023 03:02

mkoeppe added 7 commits December 26, 2023 08:52

src/sage/doctest/forker.py: Show '# [failed in baseline]' earlier

e944a54

Refactor through new function DocTestController.source_baseline

91449f7

src/sage/doctest/control.py: Fix doctest

60f056c

src/sage/doctest/control.py: Log 'Using --baseline-stats-path=...'

3b9eaa4

src/bin/sage-runtests: Generalize argparse hack

235824b

DocTestReporter.report_head: Move all printing of 'failed in baseline…

719f8d0

…' here

src/sage/doctest/reporting.py: Make 'AlarmInterrupt in doctesting fra…

19f64d4

…mework' baseline-aware

mkoeppe force-pushed the doctester-baselinestats-improvement branch from 0bfd2c9 to 19f64d4 Compare December 26, 2023 16:53

kwankyu reviewed Dec 27, 2023

View reviewed changes

src/sage/doctest/reporting.py: Improve comment

cf9024f

mkoeppe force-pushed the doctester-baselinestats-improvement branch from f61cb9a to cf9024f Compare December 27, 2023 02:38

kwankyu approved these changes Dec 27, 2023

View reviewed changes

kwankyu added s: positive review and removed s: needs review labels Dec 27, 2023

vbraun merged commit 8e8c4e4 into sagemath:develop Jan 14, 2024
17 of 21 checks passed

mkoeppe added this to the sage-10.3 milestone Jan 14, 2024

github-actions bot removed the s: positive review label Jan 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`src/sage/doctest/forker.py`: Show '# [failed in baseline]' earlier #36936

`src/sage/doctest/forker.py`: Show '# [failed in baseline]' earlier #36936

mkoeppe commented Dec 20, 2023 •

edited

Loading

kwankyu commented Dec 21, 2023 •

edited

Loading

mkoeppe commented Dec 21, 2023

kwankyu commented Dec 21, 2023

kwankyu commented Dec 21, 2023

mkoeppe commented Dec 21, 2023

mkoeppe commented Dec 21, 2023

kwankyu commented Dec 21, 2023

kwankyu commented Dec 21, 2023 •

edited

Loading

mkoeppe commented Dec 21, 2023

kwankyu commented Dec 21, 2023

kwankyu commented Dec 21, 2023

mkoeppe commented Dec 21, 2023

mkoeppe commented Dec 21, 2023

kwankyu commented Dec 21, 2023

mkoeppe commented Dec 21, 2023

kwankyu commented Dec 21, 2023

kwankyu commented Dec 21, 2023

mkoeppe commented Dec 21, 2023

kwankyu commented Dec 21, 2023

mkoeppe commented Dec 21, 2023

mkoeppe commented Dec 21, 2023

mkoeppe commented Dec 21, 2023

mkoeppe commented Dec 26, 2023

kwankyu Dec 27, 2023

mkoeppe Dec 27, 2023

kwankyu Dec 27, 2023

kwankyu Dec 27, 2023

mkoeppe Dec 27, 2023

kwankyu commented Dec 27, 2023

github-actions bot commented Dec 27, 2023

kwankyu left a comment

mkoeppe commented Dec 27, 2023

src/sage/doctest/forker.py: Show '# [failed in baseline]' earlier #36936

src/sage/doctest/forker.py: Show '# [failed in baseline]' earlier #36936

Conversation

mkoeppe commented Dec 20, 2023 • edited Loading

📝 Checklist

⌛ Dependencies

kwankyu commented Dec 21, 2023 • edited Loading

mkoeppe commented Dec 21, 2023

kwankyu commented Dec 21, 2023

kwankyu commented Dec 21, 2023

mkoeppe commented Dec 21, 2023

mkoeppe commented Dec 21, 2023

kwankyu commented Dec 21, 2023

kwankyu commented Dec 21, 2023 • edited Loading

mkoeppe commented Dec 21, 2023

kwankyu commented Dec 21, 2023

kwankyu commented Dec 21, 2023

mkoeppe commented Dec 21, 2023

mkoeppe commented Dec 21, 2023

kwankyu commented Dec 21, 2023

mkoeppe commented Dec 21, 2023

kwankyu commented Dec 21, 2023

kwankyu commented Dec 21, 2023

mkoeppe commented Dec 21, 2023

kwankyu commented Dec 21, 2023

mkoeppe commented Dec 21, 2023

mkoeppe commented Dec 21, 2023

mkoeppe commented Dec 21, 2023

mkoeppe commented Dec 26, 2023

kwankyu Dec 27, 2023

Choose a reason for hiding this comment

mkoeppe Dec 27, 2023

Choose a reason for hiding this comment

kwankyu Dec 27, 2023

Choose a reason for hiding this comment

kwankyu Dec 27, 2023

Choose a reason for hiding this comment

mkoeppe Dec 27, 2023

Choose a reason for hiding this comment

kwankyu commented Dec 27, 2023

github-actions bot commented Dec 27, 2023

kwankyu left a comment

Choose a reason for hiding this comment

mkoeppe commented Dec 27, 2023

`src/sage/doctest/forker.py`: Show '# [failed in baseline]' earlier #36936

`src/sage/doctest/forker.py`: Show '# [failed in baseline]' earlier #36936

mkoeppe commented Dec 20, 2023 •

edited

Loading

kwankyu commented Dec 21, 2023 •

edited

Loading

kwankyu commented Dec 21, 2023 •

edited

Loading