Our github checks...

133 views
Skip to first unread message

Kwankyu Lee

unread,
Oct 10, 2023, 10:29:01 AM10/10/23
to sage-devel
Hi,

I am sorry to say this, but our github checks seem to be a mess.

(1) There are two many checks. I have to scroll.
(2) Some checks look similar. I have to think to see which one is significant.
(3) They are not sorted. I don't know the order.
(4) Some checks are cancelled or skipped. I don't know why or what to do with them.
(5) Some checks are broken at the base. I have to check the check.

I started to ignore them and  to check the PR branches on my own system locally, as in the old days.

Any idea to improve the situation?


Matthias Koeppe

unread,
Oct 10, 2023, 12:22:57 PM10/10/23
to sage-devel
On Tuesday, October 10, 2023 at 7:29:01 AM UTC-7 Kwankyu Lee wrote:
I am sorry to say this, but our github checks seem to be a mess.

Yes, there are numerous issues. 

(1) There are two many checks. I have to scroll.
(2) Some checks look similar. I have to think to see which one is significant.
(3) They are not sorted. I don't know the order.

To improve (1)–(3), https://github.blog/2022-05-09-supercharging-github-actions-with-job-summaries/ may be a good approach. A PR would be most welcome. I don't plan to work on it myself, but I'll be happy to help if someone would like to take the lead on this.
 
(4) Some checks are cancelled or skipped. I don't know why or what to do with them.

It is safe to ignore skipped checks.

Checks are canceled when they take longer than 6 hours. There are unfortunately several situations in which this happens during our tests. One that is well understood:
- Downloading upstream tarballs can hang indefinitely when using unreliable mirrors. https://github.com/sagemath/sage/pull/36435 and https://github.com/sagemath/sage/pull/36438 are going to improve this; work on https://github.com/sagemath/sage/issues/34411 would also be a welcome improvement.

(5) Some checks are broken at the base. I have to check the check.

Indeed. Currently consistently broken:

sage -t --long --random-seed=286735480429121101562228604801325644303 sage/doctest/test.py  # 1 doctest failed
sage -t --long --random-seed=286735480429121101562228604801325644303 sage/parallel/decorate.py  # 1 doctest failed

https://github.com/sagemath/sage/wiki/Sage-10.2-Release-Tour#open-blocker-prs-are-applied-automatically-in-ci-workflows makes it possible for simple fixes of failures like this to take immediate effect during the same release cycle. I hope that this motivates more developers to analyze and fix these failures, rather than hoping that failures of this kind will just go away "when the next beta arrives in a week". 

Kwankyu Lee

unread,
Oct 11, 2023, 7:24:41 PM10/11/23
to sage-devel
Someone who had the same problems made a Chrome extension:

Frédéric Chapoton

unread,
Oct 15, 2023, 12:06:30 PM10/15/23
to sage-devel
now it's a step related to codecov that takes more than 1h50 minutes, see

Tobia...@gmx.de

unread,
Oct 16, 2023, 11:50:27 AM10/16/23
to sage-devel
I've now set some of the github checks as "required", so they get a small tag in the checks list. That should take care of (2).

(Hopefully, it doesn't break Volker's workflow - it shouldn't, because he has the rights to overwrite any branch protection rules.)

Kwankyu Lee

unread,
Oct 16, 2023, 6:21:58 PM10/16/23
to sage-devel
On Tuesday, October 17, 2023 at 12:50:27 AM UTC+9 Tobia...@gmx.de wrote:
I've now set some of the github checks as "required", so they get a small tag in the checks list. That should take care of (2).

I am not sure if that helps or makes things worse, as that is one more thing to look and think.

Having some checks with "Required" and some without makes things look more complicated. What should we think of a check not passed and not "Required". Should we ignore it? We should aim at "requiring" all checks passed (or skipped).

Personally I am using the chrome extension mentioned above. That solves problems (1) and (3), and (2) is less a problem for me. The problems (1), (2), (3) seem basically problems that GitHub should solve by redesigning the checks list. I hope that they adopt the design of the chrome extension.


 

Oscar Benjamin

unread,
Oct 16, 2023, 6:40:32 PM10/16/23
to sage-...@googlegroups.com
On Mon, 16 Oct 2023 at 23:22, Kwankyu Lee <ekwa...@gmail.com> wrote:
>
> On Tuesday, October 17, 2023 at 12:50:27 AM UTC+9 Tobia...@gmx.de wrote:
>
> I've now set some of the github checks as "required", so they get a small tag in the checks list. That should take care of (2).
>
> I am not sure if that helps or makes things worse, as that is one more thing to look and think.
>
> Having some checks with "Required" and some without makes things look more complicated. What should we think of a check not passed and not "Required". Should we ignore it? We should aim at "requiring" all checks passed (or skipped).

Marking checks as "required" is a fix for (5) not (2):

> (5) Some checks are broken at the base. I have to check the check.

If a job is required then a PR cannot be merged if the job fails with
the changes in the PR. Longer term that should mean that you can
depend on checks not being broken at the base because any PR that
might have broken those checks would not have been merged into the
base.

In general I would suggest aiming for having a (smaller if necessary)
set of checks that is always expected to pass unless a PR has broken
something. That can gradually be expanded but it is better if the
general expectation is that checks pass when nothing new is broken.
Currently Sage CI is just all red crosses everywhere which makes it
difficult to see whether any changes are fixing or breaking anything.

--
Oscar
Reply all
Reply to author
Forward
0 new messages