From ca5f3028763f82dfe2491d3e408afbc369ec2a73 Mon Sep 17 00:00:00 2001
From: wxiaoguang <wxiaoguang@gmail.com>
Date: Mon, 29 May 2023 23:00:21 +0800
Subject: [PATCH] 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 ++++
 options/locale/locale_en-US.ini        |  2 --
 routers/common/errpage.go              |  2 +-
 routers/common/errpage_test.go         | 41 ++++++++++++++++++++++++++
 templates/admin/config.tmpl            | 10 -------
 tests/integration/admin_config_test.go | 23 +++++++++++++++
 6 files changed, 70 insertions(+), 13 deletions(-)
 create mode 100644 routers/common/errpage_test.go
 create mode 100644 tests/integration/admin_config_test.go

diff --git a/modules/test/utils.go b/modules/test/utils.go
index 7686d3f289..282895eaa9 100644
--- a/modules/test/utils.go
+++ b/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>`)
+}
diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini
index 2399410287..e4fb50bbcb 100644
--- a/options/locale/locale_en-US.ini
+++ b/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
diff --git a/routers/common/errpage.go b/routers/common/errpage.go
index 4cf3bf8357..3d82c96deb 100644
--- a/routers/common/errpage.go
+++ b/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))
 		}
 	}()
 
diff --git a/routers/common/errpage_test.go b/routers/common/errpage_test.go
new file mode 100644
index 0000000000..ea9a9e745c
--- /dev/null
+++ b/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("..", ".."),
+	})
+}
diff --git a/templates/admin/config.tmpl b/templates/admin/config.tmpl
index 629632a718..0c52830ab0 100644
--- a/templates/admin/config.tmpl
+++ b/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>
diff --git a/tests/integration/admin_config_test.go b/tests/integration/admin_config_test.go
new file mode 100644
index 0000000000..860a92d6a3
--- /dev/null
+++ b/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()))
+}