CI Conda: Add to known-test-failures #36937

mkoeppe · 2023-12-20T22:11:30Z

Not all of the added failures are conda-specific, but the 6 instances of CI Conda amplify the error probability (#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

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 src/sage/doctest/forker.py: Show '# [failed in baseline]' earlier #36936 (which refactors the [failed in baseline] printing)

tobiasdiez · 2023-12-21T01:48:09Z

Ignoring the whole file just because there is a single failing test runs against the goal of making the conda tests more stable. Hence motion to close. For the same reason this is not suitable as blocker.

mkoeppe · 2023-12-21T01:54:39Z

Ignoring the whole file just because there is a single failing test

That's not at all what it does, as we have explained to you at length in #36678.

We have also explained to you at length why your approach of #36372 is not suitable.
It simply has no support by any reviewer.
It is highly inappropriate to use the blocker label to have it take effect in every PR.

mkoeppe · 2023-12-21T06:27:07Z

As I said in #36372 (comment) and before, Tobias, these continued abuses of your Triage team privileges have to stop.

kwankyu · 2023-12-21T07:15:09Z

I think that for each record of a file in .github/workflows/ci-conda-known-test-failures.json, there should be a github issue associated with it so that we track the history of the failures in the file.

Is there an issue for

    "sage.parallel.map_reduce": {
        "failed": true
    },

?

kwankyu · 2023-12-21T07:17:12Z

and for

    "sage.structure.coerce_actions": {
        "failed": true
    },

?

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

Since json does not support comments, we may add an "issue" field so that "issue: 12345" will point to the github issue with the number.

mkoeppe · 2023-12-21T07:41:07Z

fine with me, but I think i'd prefer a free-form text that can contain an URL or whatever for flexibility.

kwankyu · 2023-12-21T07:42:45Z

Yes, URL would work better.

mkoeppe · 2023-12-21T07:42:55Z

i added this info where available to the commit messages, so you can see it via "git blame"

kwankyu · 2023-12-21T07:44:57Z

fine with me, but I think i'd prefer a free-form text that can contain an URL or whatever for flexibility.

Then

    "sage.structure.coerce_actions": {
        "failed": true
        "comment": "some text, preferably an URL"
    },

for example.

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

i added this info where available to the commit messages, so you can see it via "git blame"

My git blame only shows the sha and the committer's name and the date. But I can see them in the git log (just for this PR).

kwankyu · 2023-12-21T07:58:59Z

@tobiasdiez Let's go this way. You may want to add those comments.

Matthias is working in #36558 to make these failures in baseline more visible to developers (without failing checks).

tobiasdiez · 2023-12-21T08:29:01Z

Yeah sure, why wouldn't we want to exclude even more files from the conda tests? For consistency, you should also add a similar file-wide exclusion mechanism for the tests that are known to fail sometimes in the Build & Test.

So stupid, instead of viewing the conda environment as a means to harden the test suit, we now massively swap the failures under the carpet. And that in a way that doesn't even allow to answer the simple question "is this the only example that fails, or are other examples failing as well"!

I also don't understand why I should add these comments. My solution keeps very well track of what example fails. It's one of the major shortcomings of Mathias solution, as I've pointed out multiple times. So at the very least he should do the work himself.

mkoeppe · 2023-12-21T16:30:02Z

You are repeating points that have already been refuted in the original discussion.

mkoeppe · 2023-12-21T19:54:18Z

For consistency, you should also add a similar file-wide exclusion mechanism for the tests that are known to fail sometimes in the Build & Test.

Yes, we may want to do this at some point. But not in this PR.

mkoeppe · 2023-12-21T20:12:13Z

Then

    "sage.structure.coerce_actions": {
        "failed": true
        "comment": "some text, preferably an URL"
    },

for example.

I'm instead just putting it in "failed" instead of true.

mkoeppe · 2023-12-22T16:51:07Z

Tobias, stop this nonsense.

tobiasdiez · 2023-12-23T16:10:22Z

.github/workflows/ci-conda-known-test-failures.json

    },
    "sage.combinat.cluster_algebra_quiver.quiver": {
-        "failed": true
+        "failed": "failure seen in https://github.com/sagemath/sage/actions/runs/6836592771/job/18591690058#step:11:10059"


These comments are pretty useless as the logs are deleted quite quickly (90 days?)

Yes, 90 days. Enough time for people to investigate and open Issues / PRs.

Not helpful to demand that it's done here.

You missed step 2 of your own procedure (#36678 (comment)):

Test failure is noticed You record it in a GitHub Issue. There you include the necessary information so that it does not take 2 hours to reconstruct it. One adds stuff to the exclusion list to eliminate noise in the tests. It's not for humans to read, and definitely not for humans to use 2 hours of guessing time to reconstruct the failure. The exclusion list is maintained like everything else in Sage -- by PRs, not by specific people in charge.

tobiasdiez · 2023-12-23T16:13:47Z

Removing "blocker" in favor of #36372 as this PR here still contains a lot of outdated exclusions.

Moreover, I would like to ask you to add these exceptions less literately. With #36372 I only added them once I had two independent PRs that show a given error. Here is my list of failures that I've seen only once:

sage -t --random-seed=214436596912198344130626509908513694044 src/sage/misc/latex.py  # 1 doctest failed

sage -t --random-seed=295417759961273984682997709208968154147 src/sage/rings/finite_rings/integer_mod.pyx  # Timed out (macos?)

sage -t --random-seed=329096063982609013558495029873741838049 src/sage/interfaces/giac.py  # 1 doctest failed

 sage -t --random-seed=128826216443334456408110059374142917558 src/sage/rings/number_field/number_field_element_quadratic.pyx  # 1 doctest failed

sage -t --random-seed=92781704964877147605342482066320289765 src/sage/schemes/elliptic_curves/ell_rational_field.py  # 1 doctest failed

sage -t --random-seed=142732370142697541280655968131944483835 src/sage/graphs/generic_graph.py  # Timed out (on macos)

sage -t --random-seed=145162806241242408148129167207410356033 src/sage/matrix/matrix2.pyx  # Timed out

sage -t --random-seed=114726989967833111405585804986834164322 src/doc/en/constructions/calculus.rst  # Timed out (on macos)

 sage -t --random-seed=3369460376730320sage -t --random-seed=73515522912035769254887028411671928367 src/sage/schemes/elliptic_curves/ell_field.py
**********************************************************************
File "src/sage/schemes/elliptic_curves/ell_field.py", line 1029, in sage.schemes.elliptic_curves.ell_field.EllipticCurve_field.division_field
Failed example:
    K = E.division_field(l)13006113016408769185426 src/sage/rings/polynomial/pbori/pbori.pyx  # 1 doctest failed
on macos

mkoeppe · 2024-01-01T02:31:24Z

only exclude the files touched in Introduce os-dependent doctest tags # known bug: macos, # known bug: linux #36960. All other files are passing (in my experience).

The point with these random errors is that no single developer's "experience" is sufficient to say that; we want to eliminate the noise from sporadic, annoying failures.

tobiasdiez · 2024-01-01T13:03:42Z

only exclude the files touched in Introduce os-dependent doctest tags # known bug: macos, # known bug: linux #36960. All other files are passing (in my experience).

The point with these random errors is that no single developer's "experience" is sufficient to say that; we want to eliminate the noise from sporadic, annoying failures.

Right, and there never have been any sporadic failures in sage.misc.lazy_import or sage.libs.singular.singular (among others); moreover a few other sporadic failures likely have been fixed in the mean time - but since you constantly remove the blocker label from #36960 it is hard to confirm this. So please either don't touch the blocker label or change this PR accordingly
(or go through all conda logs and see if the errors still appear but are suppressed).

mkoeppe · 2024-01-01T17:52:27Z

The point here is that we reduce the noise for all developers on unrelated PRs from the conda workflows.
Developers currently do not get any value from the conda workflows, only noise.
Marking things as baseline failures, even it marks more than necessary, is a mild and unproblematic solution; milder than just administratively disabling the conda workflow.
That's why this PR here is a blocker, and it should be merged ASAP.

The concern of one developer to confirm whether the failures marked up in #36960 cover everything is secondary. Monitoring this can be done just using the logs and does not have to rely on signaling test failures to all other developers.

tobiasdiez · 2024-01-04T16:20:44Z

The concern of one developer to confirm whether the failures marked up in #36960 cover everything is secondary. Monitoring this can be done just using the logs and does not have to rely on signaling test failures to all other developers.

I now went through the logs and added a few more exceptions to #36960. So every failure (that occurred more than once) is now excluded in #36860. Could you please mirror the changes here. Thanks!

mkoeppe · 2024-01-04T16:55:55Z

Completeness is not part of the scope of this PR.

mkoeppe · 2024-01-04T17:15:23Z

I now went through the logs and added a few more exceptions to #36960.

Thanks for doing this work. Do you have a script for this - retrieving and parsing logs?

You may be interested in:

CI: Upload test stats as artifacts, improve output of "List Docker images" #36970

sagemath#36832

…s from sagemath#36372

…rom commit messages

…agemath#36372

…e]' if provided

github-actions · 2024-01-22T05:57:42Z

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

mkoeppe · 2024-01-23T21:22:38Z

Let's merge this please.

kwankyu

OK. Hopefully save us from chronic conda failures in CI.

sagemathgh-36937: CI Conda: Add to known-test-failures 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

tobiasdiez added r: invalid and removed p: blocker / 1 labels Dec 21, 2023

tobiasdiez added r: invalid and removed p: blocker / 1 labels Dec 22, 2023

kwankyu mentioned this pull request Dec 22, 2023

Fix/ignore all remaining test failures when run under conda #36372

Closed

5 tasks

tobiasdiez reviewed Dec 23, 2023

View reviewed changes

tobiasdiez removed p: blocker / 1 s: needs review labels Dec 23, 2023

tobiasdiez added s: needs work and removed s: needs review labels Jan 1, 2024

tobiasdiez added s: needs work and removed s: needs review labels Jan 4, 2024

Matthias Koeppe added 9 commits January 21, 2024 17:04

.github/workflows/ci-conda-known-test-failures.json: Add random failure

3bfed13

sagemath#36832

.github/workflows/ci-conda-known-test-failures.json: Add sagemath#36939

d803c6c

.github/workflows/ci-conda-known-test-failures.json: Add sagemath#35973

255fc6a

.github/workflows/ci-conda-known-test-failures.json: Add some failure…

410bc13

…s from sagemath#36372

.github/workflows/ci-conda-known-test-failures.json: Add references f…

4d3d50e

…rom commit messages

.github/workflows/ci-conda-known-test-failures.json: Add details from s…

9db526c

…agemath#36372

src/sage/doctest/reporting.py: Include message in '[failed in baselin…

5e69712

…e]' if provided

.github/workflows/ci-conda-known-test-failures.json: Add sagemath#36949

fa6ec3d

.github/workflows/ci-conda-known-test-failures.json: Add sagemath#28559

8f4bc66

kwankyu approved these changes Jan 23, 2024

View reviewed changes

kwankyu added s: positive review and removed s: needs review labels Jan 23, 2024

vbraun merged commit caf7cf8 into sagemath:develop Feb 2, 2024

github-actions bot removed the s: positive review label Feb 2, 2024

Uh oh!

CI Conda: Add to known-test-failures #36937

CI Conda: Add to known-test-failures #36937

Uh oh!

Conversation

mkoeppe commented Dec 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📝 Checklist

⌛ Dependencies

Uh oh!

tobiasdiez commented Dec 21, 2023

Uh oh!

mkoeppe commented Dec 21, 2023

Uh oh!

mkoeppe commented Dec 21, 2023

Uh oh!

kwankyu commented Dec 21, 2023

Uh oh!

kwankyu commented Dec 21, 2023

Uh oh!

kwankyu commented Dec 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mkoeppe commented Dec 21, 2023

Uh oh!

kwankyu commented Dec 21, 2023

Uh oh!

mkoeppe commented Dec 21, 2023

Uh oh!

kwankyu commented Dec 21, 2023

Uh oh!

kwankyu commented Dec 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kwankyu commented Dec 21, 2023

Uh oh!

tobiasdiez commented Dec 21, 2023

Uh oh!

mkoeppe commented Dec 21, 2023

Uh oh!

mkoeppe commented Dec 21, 2023

Uh oh!

mkoeppe commented Dec 21, 2023

Uh oh!

mkoeppe commented Dec 22, 2023

Uh oh!

tobiasdiez Dec 23, 2023

Choose a reason for hiding this comment

Uh oh!

mkoeppe Jan 1, 2024

Choose a reason for hiding this comment

Uh oh!

tobiasdiez Jan 4, 2024

Choose a reason for hiding this comment

Uh oh!

tobiasdiez commented Dec 23, 2023

Uh oh!

mkoeppe commented Jan 1, 2024

Uh oh!

tobiasdiez commented Jan 1, 2024

Uh oh!

mkoeppe commented Jan 1, 2024

Uh oh!

tobiasdiez commented Jan 4, 2024

Uh oh!

mkoeppe commented Jan 4, 2024

Uh oh!

mkoeppe commented Jan 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jan 22, 2024

Uh oh!

mkoeppe commented Jan 23, 2024

Uh oh!

kwankyu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mkoeppe commented Dec 20, 2023 •

edited

Loading

kwankyu commented Dec 21, 2023 •

edited

Loading

kwankyu commented Dec 21, 2023 •

edited

Loading

mkoeppe commented Jan 4, 2024 •

edited

Loading