From d97e36f6d724313ae729dcccdab9482829a02b4f Mon Sep 17 00:00:00 2001
From: Otto Richter <git@otto.splvs.net>
Date: Sun, 25 Aug 2024 14:52:21 +0200
Subject: [PATCH 1/2] Playwright testing for commit diffs
includes:
- easier repo declaration for playwright tests by @Gusted
- full backend build for pushing Git repos by @Gusted
- playwright testing (which fails with the current diff algorithm, but
passes with the new)
- disable eslint rule for conditional expect, because it defeats the
purpose (working around it would result in much more complex test code
in our cases)
---
.forgejo/workflows/e2e.yml | 4 +-
tests/e2e/.eslintrc.yaml | 1 +
tests/e2e/debugserver_test.go | 3 +-
tests/e2e/declare_repos_test.go | 83 +++++++++++++++++++++++++++++++++
tests/e2e/e2e_test.go | 5 +-
tests/e2e/repo-code.test.e2e.js | 26 +++++++++++
tests/e2e/utils_e2e_test.go | 6 +--
7 files changed, 121 insertions(+), 7 deletions(-)
create mode 100644 tests/e2e/declare_repos_test.go
diff --git a/.forgejo/workflows/e2e.yml b/.forgejo/workflows/e2e.yml
index 3cf3d8641..949177eb8 100644
--- a/.forgejo/workflows/e2e.yml
+++ b/.forgejo/workflows/e2e.yml
@@ -22,7 +22,7 @@ jobs:
go-version-file: "go.mod"
- run: |
apt-get -qq update
- apt-get -qq install -q sudo
+ apt-get -qq install -q sudo git git-lfs
sed -i -e 's/%sudo.*/%sudo ALL=(ALL:ALL) NOPASSWD:ALL/' /etc/sudoers
git config --add safe.directory '*'
adduser --quiet --comment forgejo --disabled-password forgejo
@@ -30,6 +30,8 @@ jobs:
chown -R forgejo:forgejo .
- run: |
su forgejo -c 'make deps-frontend frontend deps-backend'
+ - run: |
+ su forgejo -c 'make backend'
- run: |
su forgejo -c 'make generate test-e2e-sqlite'
timeout-minutes: 40
diff --git a/tests/e2e/.eslintrc.yaml b/tests/e2e/.eslintrc.yaml
index 390b2de5c..148643152 100644
--- a/tests/e2e/.eslintrc.yaml
+++ b/tests/e2e/.eslintrc.yaml
@@ -14,6 +14,7 @@ env:
rules:
playwright/no-conditional-in-test: [0]
+ playwright/no-conditional-expect: [0]
playwright/no-networkidle: [0]
playwright/no-skipped-test: [2, {allowConditional: true}]
playwright/prefer-comparison-matcher: [2]
diff --git a/tests/e2e/debugserver_test.go b/tests/e2e/debugserver_test.go
index b1496b22a..49461fabe 100644
--- a/tests/e2e/debugserver_test.go
+++ b/tests/e2e/debugserver_test.go
@@ -24,7 +24,8 @@ func TestDebugserver(t *testing.T) {
done := make(chan os.Signal, 1)
signal.Notify(done, syscall.SIGINT, syscall.SIGTERM)
- onGiteaRun(t, func(*testing.T, *url.URL) {
+ onForgejoRun(t, func(*testing.T, *url.URL) {
+ defer DeclareGitRepos(t)()
fmt.Println(setting.AppURL)
<-done
})
diff --git a/tests/e2e/declare_repos_test.go b/tests/e2e/declare_repos_test.go
new file mode 100644
index 000000000..44ee79f38
--- /dev/null
+++ b/tests/e2e/declare_repos_test.go
@@ -0,0 +1,83 @@
+// Copyright 2024 The Forgejo Authors. All rights reserved.
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package e2e
+
+import (
+ "fmt"
+ "strconv"
+ "strings"
+ "testing"
+ "time"
+
+ unit_model "code.gitea.io/gitea/models/unit"
+ "code.gitea.io/gitea/models/unittest"
+ user_model "code.gitea.io/gitea/models/user"
+ "code.gitea.io/gitea/modules/git"
+ files_service "code.gitea.io/gitea/services/repository/files"
+ "code.gitea.io/gitea/tests"
+
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+)
+
+type FileChanges [][]string
+
+// put your Git repo declarations in here
+// feel free to amend the helper function below or use the raw variant directly
+func DeclareGitRepos(t *testing.T) func() {
+ var cleanupFunctions []func()
+ cleanupFunctions = append(cleanupFunctions, newRepo(t, 2, "diff-test", FileChanges{
+ {"testfile", "hello", "hallo", "hola", "native", "ubuntu-latest", "- runs-on: ubuntu-latest", "- runs-on: debian-latest"},
+ }))
+
+ return func() {
+ for _, cleanup := range cleanupFunctions {
+ cleanup()
+ }
+ }
+}
+
+func newRepo(t *testing.T, userID int64, repoName string, fileChanges FileChanges) func() {
+ user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: userID})
+ somerepo, _, cleanupFunc := tests.CreateDeclarativeRepo(t, user, repoName,
+ []unit_model.Type{unit_model.TypeCode, unit_model.TypeIssues}, nil,
+ nil,
+ )
+
+ for _, file := range fileChanges {
+ changeLen := len(file)
+ for i := 1; i < changeLen; i++ {
+ operation := "create"
+ if i != 1 {
+ operation = "update"
+ }
+ resp, err := files_service.ChangeRepoFiles(git.DefaultContext, somerepo, user, &files_service.ChangeRepoFilesOptions{
+ Files: []*files_service.ChangeRepoFile{{
+ Operation: operation,
+ TreePath: file[0],
+ ContentReader: strings.NewReader(file[i]),
+ }},
+ Message: fmt.Sprintf("Patch: %s-%s", file[0], strconv.Itoa(i)),
+ OldBranch: "main",
+ NewBranch: "main",
+ Author: &files_service.IdentityOptions{
+ Name: user.Name,
+ Email: user.Email,
+ },
+ Committer: &files_service.IdentityOptions{
+ Name: user.Name,
+ Email: user.Email,
+ },
+ Dates: &files_service.CommitDateOptions{
+ Author: time.Now(),
+ Committer: time.Now(),
+ },
+ })
+ require.NoError(t, err)
+ assert.NotEmpty(t, resp)
+ }
+ }
+
+ return cleanupFunc
+}
diff --git a/tests/e2e/e2e_test.go b/tests/e2e/e2e_test.go
index 39974e00c..44a6897bf 100644
--- a/tests/e2e/e2e_test.go
+++ b/tests/e2e/e2e_test.go
@@ -37,7 +37,7 @@ func TestMain(m *testing.M) {
graceful.InitManager(managerCtx)
defer cancel()
- tests.InitTest(false)
+ tests.InitTest(true)
testE2eWebRoutes = routers.NormalRoutes()
os.Unsetenv("GIT_AUTHOR_NAME")
@@ -102,7 +102,8 @@ func TestE2e(t *testing.T) {
t.Run(testname, func(t *testing.T) {
// Default 2 minute timeout
- onGiteaRun(t, func(*testing.T, *url.URL) {
+ onForgejoRun(t, func(*testing.T, *url.URL) {
+ defer DeclareGitRepos(t)()
thisTest := runArgs
thisTest = append(thisTest, path)
cmd := exec.Command(runArgs[0], thisTest...)
diff --git a/tests/e2e/repo-code.test.e2e.js b/tests/e2e/repo-code.test.e2e.js
index 626af7630..01179d28d 100644
--- a/tests/e2e/repo-code.test.e2e.js
+++ b/tests/e2e/repo-code.test.e2e.js
@@ -51,3 +51,29 @@ test('Line Range Selection', async ({browser}, workerInfo) => {
await page.goto(`${filePath}#L1-L100`);
await assertSelectedLines(page, ['1', '2', '3']);
});
+
+test('Readable diff', async ({page}, workerInfo) => {
+ // remove this when the test covers more (e.g. accessibility scans or interactive behaviour)
+ test.skip(workerInfo.project.name !== 'firefox', 'This currently only tests the backend-generated HTML code and it is not necessary to test with multiple browsers.');
+ const expectedDiffs = [
+ {id: 'testfile-2', removed: 'e', added: 'a'},
+ {id: 'testfile-3', removed: 'allo', added: 'ola'},
+ {id: 'testfile-4', removed: 'hola', added: 'native'},
+ {id: 'testfile-5', removed: 'native', added: 'ubuntu-latest'},
+ {id: 'testfile-6', added: '- runs-on: '},
+ {id: 'testfile-7', removed: 'ubuntu', added: 'debian'},
+ ];
+ for (const thisDiff of expectedDiffs) {
+ const response = await page.goto('/user2/diff-test/commits/branch/main');
+ await expect(response?.status()).toBe(200); // Status OK
+ await page.getByText(`Patch: ${thisDiff.id}`).click();
+ if (thisDiff.removed) {
+ await expect(page.getByText(thisDiff.removed, {exact: true})).toHaveClass(/removed-code/);
+ await expect(page.getByText(thisDiff.removed, {exact: true})).toHaveCSS('background-color', 'rgb(252, 165, 165)');
+ }
+ if (thisDiff.added) {
+ await expect(page.getByText(thisDiff.added, {exact: true})).toHaveClass(/added-code/);
+ await expect(page.getByText(thisDiff.added, {exact: true})).toHaveCSS('background-color', 'rgb(134, 239, 172)');
+ }
+ }
+});
diff --git a/tests/e2e/utils_e2e_test.go b/tests/e2e/utils_e2e_test.go
index a0e940f1e..cfd3ff9e3 100644
--- a/tests/e2e/utils_e2e_test.go
+++ b/tests/e2e/utils_e2e_test.go
@@ -17,7 +17,7 @@ import (
"github.com/stretchr/testify/require"
)
-func onGiteaRunTB(t testing.TB, callback func(testing.TB, *url.URL), prepare ...bool) {
+func onForgejoRunTB(t testing.TB, callback func(testing.TB, *url.URL), prepare ...bool) {
if len(prepare) == 0 || prepare[0] {
defer tests.PrepareTestEnv(t, 1)()
}
@@ -49,8 +49,8 @@ func onGiteaRunTB(t testing.TB, callback func(testing.TB, *url.URL), prepare ...
callback(t, u)
}
-func onGiteaRun(t *testing.T, callback func(*testing.T, *url.URL), prepare ...bool) {
- onGiteaRunTB(t, func(t testing.TB, u *url.URL) {
+func onForgejoRun(t *testing.T, callback func(*testing.T, *url.URL), prepare ...bool) {
+ onForgejoRunTB(t, func(t testing.TB, u *url.URL) {
callback(t.(*testing.T), u)
}, prepare...)
}
From 58ee9fdc4a217ea145ec60dd524953fff7d91743 Mon Sep 17 00:00:00 2001
From: Gusted <postmaster@gusted.xyz>
Date: Sun, 18 Aug 2024 23:26:41 +0200
Subject: [PATCH 2/2] feat: Improve diff being generated
Add `DiffCleanupSemantic` into the mix when generated diffs (PR review,
commit view and issue/comment history). This avoids trying to produce a
optimal diff and tries to reduce the amount of edits, by combing them
into larger edits, which is nicer and easier to 'look at'. There's no
need for a perfect minimal diff, as the output isn't being parsed by a
computer, it's parsed by people.
Ref: https://codeberg.org/forgejo/forgejo/issues/4996
---
routers/web/repo/issue_content_history.go | 1 +
services/gitdiff/highlightdiff.go | 1 +
2 files changed, 2 insertions(+)
diff --git a/routers/web/repo/issue_content_history.go b/routers/web/repo/issue_content_history.go
index 31d2de6d5..16b250abd 100644
--- a/routers/web/repo/issue_content_history.go
+++ b/routers/web/repo/issue_content_history.go
@@ -154,6 +154,7 @@ func GetContentHistoryDetail(ctx *context.Context) {
dmp := diffmatchpatch.New()
// `checklines=false` makes better diff result
diff := dmp.DiffMain(prevHistoryContentText, history.ContentText, false)
+ diff = dmp.DiffCleanupSemantic(diff)
diff = dmp.DiffCleanupEfficiency(diff)
// use chroma to render the diff html
diff --git a/services/gitdiff/highlightdiff.go b/services/gitdiff/highlightdiff.go
index 99313c5f3..c72959ea1 100644
--- a/services/gitdiff/highlightdiff.go
+++ b/services/gitdiff/highlightdiff.go
@@ -97,6 +97,7 @@ func (hcd *HighlightCodeDiff) diffWithHighlight(filename, language, codeA, codeB
convertedCodeB := hcd.ConvertToPlaceholders(string(highlightCodeB))
diffs := diffMatchPatch.DiffMain(convertedCodeA, convertedCodeB, true)
+ diffs = diffMatchPatch.DiffCleanupSemantic(diffs)
diffs = diffMatchPatch.DiffCleanupEfficiency(diffs)
for i := range diffs {