Fix admin config page error, use tests to cover the admin config and …

…500 error page (#24965)

The admin config page has been broken for many many times, a little
refactoring would make this page panic.

So, add a test for it, and add another test to cover the 500 error page.

Co-authored-by: Giteabot <teabot@gitea.io>

modules/test/utils.go

@@ -5,9 +5,14 @@ package test
 
 import (
 	"net/http"
+	"strings"
 )
 
 // RedirectURL returns the redirect URL of a http response.
 func RedirectURL(resp http.ResponseWriter) string {
 	return resp.Header().Get("Location")
 }
+
+func IsNormalPageCompleted(s string) bool {
+	return strings.Contains(s, `<footer class="page-footer"`) && strings.Contains(s, `</html>`)
+}

options/locale/locale_en-US.ini

@@ -3039,8 +3039,6 @@ config.git_disable_diff_highlight = Disable Diff Syntax Highlight
 config.git_max_diff_lines = Max Diff Lines (for a single file)
 config.git_max_diff_line_characters = Max Diff Characters (for a single line)
 config.git_max_diff_files = Max Diff Files (to be shown)
-config.git_enable_reflogs = Enable Reflogs
-config.git_reflog_expiry_time = Expiry Time
 config.git_gc_args = GC Arguments
 config.git_migrate_timeout = Migration Timeout
 config.git_mirror_timeout = Mirror Update Timeout

routers/common/errpage.go

@@ -26,7 +26,7 @@ func RenderPanicErrorPage(w http.ResponseWriter, req *http.Request, err any) {
 
 	defer func() {
 		if err := recover(); err != nil {
-			log.Error("Panic occurs again when rendering error page: %v", err)
+			log.Error("Panic occurs again when rendering error page: %v. Stack:\n%s", err, log.Stack(2))
 		}
 	}()
 

routers/common/errpage_test.go

@@ -0,0 +1,41 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package common
+
+import (
+	"context"
+	"errors"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"path/filepath"
+	"testing"
+
+	"code.gitea.io/gitea/models/unittest"
+	"code.gitea.io/gitea/modules/test"
+	"code.gitea.io/gitea/modules/web/middleware"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestRenderPanicErrorPage(t *testing.T) {
+	w := httptest.NewRecorder()
+	req := &http.Request{URL: &url.URL{}}
+	req = req.WithContext(middleware.WithContextData(context.Background()))
+	RenderPanicErrorPage(w, req, errors.New("fake panic error (for test only)"))
+	respContent := w.Body.String()
+	assert.Contains(t, respContent, `class="page-content status-page-500"`)
+	assert.Contains(t, respContent, `</html>`)
+
+	// the 500 page doesn't have normal pages footer, it makes it easier to distinguish a normal page and a failed page.
+	// especially when a sub-template causes page error, the HTTP response code is still 200,
+	// the different "footer" is the only way to know whether a page is fully rendered without error.
+	assert.False(t, test.IsNormalPageCompleted(respContent))
+}
+
+func TestMain(m *testing.M) {
+	unittest.MainTest(m, &unittest.TestOptions{
+		GiteaRootPath: filepath.Join("..", ".."),
+	})
+}

templates/admin/config.tmpl

@@ -333,16 +333,6 @@
 
 				<div class="ui divider"></div>
 
-				<dt>{{.locale.Tr "admin.config.git_enable_reflogs"}}</dt>
-				<dd>{{if .Git.Reflog.Enabled}}{{svg "octicon-check"}}{{else}}{{svg "octicon-x"}}{{end}}</dd>
-
-				{{if .Git.Reflog.Enabled}}
-					<dt>{{.locale.Tr "admin.config.git_reflog_expiry_time"}}</dt>
-					<dd>{{.locale.Tr "tool.days" .Git.Reflog.Expiration}}</dd>
-				{{end}}
-
-				<div class="ui divider"></div>
-
 				<dt>{{.locale.Tr "admin.config.git_migrate_timeout"}}</dt>
 				<dd>{{.Git.Timeout.Migrate}} {{.locale.Tr "tool.raw_seconds"}}</dd>
 				<dt>{{.locale.Tr "admin.config.git_mirror_timeout"}}</dt>

tests/integration/admin_config_test.go

@@ -0,0 +1,23 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package integration
+
+import (
+	"net/http"
+	"testing"
+
+	"code.gitea.io/gitea/modules/test"
+	"code.gitea.io/gitea/tests"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestAdminConfig(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+
+	session := loginUser(t, "user1")
+	req := NewRequest(t, "GET", "/admin/config")
+	resp := session.MakeRequest(t, req, http.StatusOK)
+	assert.True(t, test.IsNormalPageCompleted(resp.Body.String()))
+}