Add cache test for admins

[6543]

Add a test to probe the cache similar to the email test func.

[wxiaoguang]

It has been tested during startup.

[6543]

yes but esp if it,s a remote dep, it cold break afterwards and this is an easy way to test it

[wxiaoguang]

yes but esp if it,s a remote dep, it cold break afterwards and this is an easy way to test it

If it breaks, either there will be 500 error / error log, or Gitea won't restart. I do not think it needs to "test it" manually on the admin panel, we shouldn't test everything manually there IMO.

If it should be tested, what is the real "use case" for it?

[6543]

The "real" usecase is, as mentoined, to let gitea admins check if the cache works and if it is slow or not ...

As gitea admins do not always have sys acces to logs etc... and if you have it in prod mode the 500 page will tell you nothing! (As it should)

[wxiaoguang]

The "real" usecase is, as mentoined, to let gitea admins check if the cache works and if it is slow or not ...

• "if the cache works": it won't work in this case. Now Gitea heavily depends on the cache, I can imagine that in the futrue, if the cache doesn't work, then admin couldn't login or couldn't open your "check" page.
• "if it is slow or not": we have a "self check" page now. I think we should add various checks there, check all of them automatically, instead of adding more "check" buttons.

[wxiaoguang]

and if you have it in prod mode the 500 page will tell you nothing! (As it should)

That's not true because it always shows the detail error messages if you are an admin. And if there is really something wrong, admin should check the logs.

[6543]

sorry but I personally see it as quality of life improvement for admins, if you don't need it it's fine but don't expect all others to have the same view

[silverwind]

Personally I don't mind such troubleshooting buttons. They are good to have when the need arises.

[wxiaoguang]

Personally I don't mind such troubleshooting buttons. They are good to have when the need arises.

Why not just re-use the "self check" page? Test everything automatically.

[silverwind]

Maybe something can be done about the vertical centering of text and button in this list, I can check it out later.

[silverwind]

Personally I don't mind such troubleshooting buttons. They are good to have when the need arises.

Why not just re-use the "self check" page? Test everything automatically.

Hmm, if self-check includes this check, then we should at least make self check call this function instead of having two duplicate check functions.

[techknowlogick]

Fwiw, I don't see this as any different from the testing mail functionality, as not all code paths will fail if cache is unavailable (unlike if DB fails), and so adding the option for an admin to check would be good.

I'll wait to review until the duplication reduction mentioned by @silverwind is handled.

[6543]

@techknowlogick add the check to the serftest too ... and mv it into a dedicated func with an unit test

[silverwind]

Noticed one UI problem: If there is a error displayed on the Summary page, the test result message will not show because the error message apparently has precedence:

Either make both show or use showInfoToast and showErrorToast in JS to display the result as toast. Toast is imho better because it does not need a page reload.

To reproduce, add this to app.ini:

[log]
ROUTER = foo

[silverwind]

Regarding centering, I think it's actually better not to do it because it would cause this labels on other content to move like this:

[silverwind]

Actually I decided to center those two entries with the test functions with some tailwind, this is how it looks now:

[6543]

Noticed one UI problem: If there is a error displayed on the Summary page, the test result message will not show because the error message apparently has precedence:

...

is this not out of scope of this pull 😆

[silverwind]

Noticed one UI problem: If there is a error displayed on the Summary page, the test result message will not show because the error message apparently has precedence:

...

is this not out of scope of this pull 😆

If it affects test email too, I'd say out of scope. I assume the bug is in backend flash somewhere. At least the template code has the ability to render one flash per severity level. Imho it needs a ability to render more than one flash per severity but that is definitly out of scope 😆.

[wxiaoguang]

Noticed one UI problem: If there is a error displayed on the Summary page, the test result message will not show because the error message apparently has precedence:

...

is this not out of scope of this pull 😆

If it affects test email too, I'd say out of scope. I assume the bug is in backend flash somewhere. At least the template code has the ability to render one flash per severity level. Imho it needs a ability to render more than one flash per severity but that is definitly out of scope 😆.

That's why it should use "self check" page to test it automaticaly, instead of adding more "check" buttons (no more flash)

[silverwind]

It's still not part of /admin/self_check? I was under the impression it was. I don't mind having the button to trigger it individually but if it is part of the self check, it's certainly kind of redundant.

[6543]

well it is indeed usefull to have it in the selve test but I realy want to have the manuall feedback for this specific part of gitea for admins ... and as we now just use the same func there is no much dublicated code at all

[wxiaoguang]

It's still not part of /admin/self_check? I was under the impression it was. I don't mind having the button to trigger it individually but if it is part of the self check, it's certainly kind of redundant.

I didn't mean block, I could refactor&improve the self-check page later.

[silverwind]

Well, let's take it then, to be hopefully improved later.