From 634519e891f7f7f9831ed365fd51f47649b8cb19 Mon Sep 17 00:00:00 2001
From: 0ko <0ko@noreply.codeberg.org>
Date: Sun, 10 Nov 2024 17:57:32 +0500
Subject: [PATCH 1/4] feat(ui): highlight user mention in comments and commit
messages
---
templates/base/head.tmpl | 1 +
templates/shared/user/mention_highlight.tmpl | 14 +++++++++++++
tests/integration/mention_test.go | 21 ++++++++++++++++++++
3 files changed, 36 insertions(+)
create mode 100644 templates/shared/user/mention_highlight.tmpl
create mode 100644 tests/integration/mention_test.go
diff --git a/templates/base/head.tmpl b/templates/base/head.tmpl
index 7753f49243..9eb5b5addf 100644
--- a/templates/base/head.tmpl
+++ b/templates/base/head.tmpl
@@ -26,6 +26,7 @@
.ui.secondary.menu .dropdown.item > .menu { margin-top: 0; }
+ {{template "shared/user/mention_highlight" .}}
{{template "base/head_opengraph" .}}
{{template "base/head_style" .}}
{{template "custom/header" .}}
diff --git a/templates/shared/user/mention_highlight.tmpl b/templates/shared/user/mention_highlight.tmpl
new file mode 100644
index 0000000000..1551cef874
--- /dev/null
+++ b/templates/shared/user/mention_highlight.tmpl
@@ -0,0 +1,14 @@
+{{if .IsSigned}}
+
+{{end}}
diff --git a/tests/integration/mention_test.go b/tests/integration/mention_test.go
new file mode 100644
index 0000000000..36a0ccb312
--- /dev/null
+++ b/tests/integration/mention_test.go
@@ -0,0 +1,21 @@
+// Copyright 2024 The Forgejo Authors. All rights reserved.
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package integration
+
+import (
+ "net/http"
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+)
+
+func TestHeadMentionCSS(t *testing.T) {
+ userSession := loginUser(t, "user2")
+ resp := userSession.MakeRequest(t, NewRequest(t, "GET", "/"), http.StatusOK)
+ assert.Contains(t, resp.Body.String(), `.mention[href="/user2" i]`)
+
+ guestSession := emptyTestSession(t)
+ resp = guestSession.MakeRequest(t, NewRequest(t, "GET", "/"), http.StatusOK)
+ assert.NotContains(t, resp.Body.String(), `.mention[href="`)
+}
From c17b4bdaeb3134ac281e35199929d02a2caf19e7 Mon Sep 17 00:00:00 2001
From: Otto Richter
Date: Mon, 11 Nov 2024 18:44:55 +0100
Subject: [PATCH 2/4] tests(e2e): Separate accessibility and form checks
- automatically test for light and dark themes
---
tests/e2e/shared/accessibility.ts | 35 +++++++++++++++++++++++++++++++
tests/e2e/shared/forms.ts | 15 ++++++-------
2 files changed, 41 insertions(+), 9 deletions(-)
create mode 100644 tests/e2e/shared/accessibility.ts
diff --git a/tests/e2e/shared/accessibility.ts b/tests/e2e/shared/accessibility.ts
new file mode 100644
index 0000000000..6675e0d9eb
--- /dev/null
+++ b/tests/e2e/shared/accessibility.ts
@@ -0,0 +1,35 @@
+import {expect, type Page} from '@playwright/test';
+import {AxeBuilder} from '@axe-core/playwright';
+
+export async function accessibilityCheck({page}: {page: Page}, includes: string[], excludes: string[], disabledRules: string[]) {
+ // contrast of inline links is still a global issue in Forgejo
+ disabledRules += 'link-in-text-block';
+
+ let accessibilityScanner = await new AxeBuilder({page})
+ .disableRules(disabledRules);
+ // passing the whole array seems to be not supported,
+ // iterating has the nice side-effectof skipping this if the array is empty
+ for (const incl of includes) {
+ // passing the whole array seems to be not supported
+ accessibilityScanner = accessibilityScanner.include(incl);
+ }
+ for (const excl of excludes) {
+ accessibilityScanner = accessibilityScanner.exclude(excl);
+ }
+
+ // scan the page both in dark and light theme
+ let accessibilityScanResults = await accessibilityScanner.analyze();
+ expect(accessibilityScanResults.violations).toEqual([]);
+ await page.emulateMedia({colorScheme: 'dark'});
+ // in https://codeberg.org/forgejo/forgejo/pulls/5899 there have been
+ // some weird failures related to contrast scanning,
+ // reporting for colours that haven't been used and no trace in the
+ // screenshots.
+ // Since this was only happening with some browsers and not always,
+ // my bet is on a transition effect on dark/light mode switch.
+ // Waiting a little seems to work around this.
+ await page.waitForTimeout(100); // eslint-disable-line playwright/no-wait-for-timeout
+ accessibilityScanResults = await accessibilityScanner.analyze();
+ expect(accessibilityScanResults.violations).toEqual([]);
+ await page.emulateMedia({colorScheme: 'light'});
+}
diff --git a/tests/e2e/shared/forms.ts b/tests/e2e/shared/forms.ts
index 52432ccbe8..2728acf5e7 100644
--- a/tests/e2e/shared/forms.ts
+++ b/tests/e2e/shared/forms.ts
@@ -1,17 +1,14 @@
import {expect, type Page} from '@playwright/test';
-import {AxeBuilder} from '@axe-core/playwright';
+import {accessibilityCheck} from './accessibility.ts';
export async function validate_form({page}: {page: Page}, scope: 'form' | 'fieldset' = 'form') {
- const accessibilityScanResults = await new AxeBuilder({page})
- // disable checking for link style - should be fixed, but not now
- .disableRules('link-in-text-block')
- .include(scope)
+ const excludedElements = [
// exclude automated tooltips from accessibility scan, remove when fixed
- .exclude('span[data-tooltip-content')
+ 'span[data-tooltip-content',
// exclude weird non-semantic HTML disabled content
- .exclude('.disabled')
- .analyze();
- expect(accessibilityScanResults.violations).toEqual([]);
+ '.disabled',
+ ];
+ await accessibilityCheck({page}, [scope], excludedElements, []);
// assert CSS properties that needed to be overriden for forms (ensure they remain active)
const boxes = page.getByRole('checkbox').or(page.getByRole('radio'));
From 1f7a64805762493b65db3ac4db7012039033384f Mon Sep 17 00:00:00 2001
From: Otto Richter
Date: Sun, 10 Nov 2024 20:09:53 +0100
Subject: [PATCH 3/4] tests(e2e): mention highlights in commit messages
---
tests/e2e/declare_repos_test.go | 48 ++++++++++++++++++++++++---------
tests/e2e/repo-code.test.e2e.ts | 23 +++++++++++++++-
2 files changed, 57 insertions(+), 14 deletions(-)
diff --git a/tests/e2e/declare_repos_test.go b/tests/e2e/declare_repos_test.go
index 7057b26b6f..c55a42ac66 100644
--- a/tests/e2e/declare_repos_test.go
+++ b/tests/e2e/declare_repos_test.go
@@ -5,7 +5,6 @@ package e2e
import (
"fmt"
- "strconv"
"strings"
"testing"
"time"
@@ -23,14 +22,31 @@ import (
// first entry represents filename
// the following entries define the full file content over time
-type FileChanges [][]string
+type FileChanges struct {
+ Filename string
+ CommitMsg string
+ Versions []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() {
cleanupFunctions := []func(){
- newRepo(t, 2, "diff-test", FileChanges{
- {"testfile", "hello", "hallo", "hola", "native", "ubuntu-latest", "- runs-on: ubuntu-latest", "- runs-on: debian-latest"},
+ newRepo(t, 2, "diff-test", []FileChanges{{
+ Filename: "testfile",
+ Versions: []string{"hello", "hallo", "hola", "native", "ubuntu-latest", "- runs-on: ubuntu-latest", "- runs-on: debian-latest"},
+ }}),
+ newRepo(t, 2, "mentions-highlighted", []FileChanges{
+ {
+ Filename: "history1.md",
+ Versions: []string{""},
+ CommitMsg: "A commit message which mentions @user2 in the title\nand has some additional text which mentions @user1",
+ },
+ {
+ Filename: "history2.md",
+ Versions: []string{""},
+ CommitMsg: "Another commit which mentions @user1 in the title\nand @user2 in the text",
+ },
}),
// add your repo declarations here
}
@@ -42,7 +58,7 @@ func DeclareGitRepos(t *testing.T) func() {
}
}
-func newRepo(t *testing.T, userID int64, repoName string, fileChanges FileChanges) func() {
+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,
@@ -50,19 +66,25 @@ func newRepo(t *testing.T, userID int64, repoName string, fileChanges FileChange
)
for _, file := range fileChanges {
- changeLen := len(file)
- for i := 1; i < changeLen; i++ {
- operation := "create"
- if i != 1 {
- operation = "update"
+ for i, version := range file.Versions {
+ operation := "update"
+ if i == 0 {
+ operation = "create"
}
+
+ // default to unique commit messages
+ commitMsg := file.CommitMsg
+ if commitMsg == "" {
+ commitMsg = fmt.Sprintf("Patch: %s-%d", file.Filename, i+1)
+ }
+
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]),
+ TreePath: file.Filename,
+ ContentReader: strings.NewReader(version),
}},
- Message: fmt.Sprintf("Patch: %s-%s", file[0], strconv.Itoa(i)),
+ Message: commitMsg,
OldBranch: "main",
NewBranch: "main",
Author: &files_service.IdentityOptions{
diff --git a/tests/e2e/repo-code.test.e2e.ts b/tests/e2e/repo-code.test.e2e.ts
index b22670ab76..d114a9b9c0 100644
--- a/tests/e2e/repo-code.test.e2e.ts
+++ b/tests/e2e/repo-code.test.e2e.ts
@@ -5,7 +5,12 @@
// @watch end
import {expect} from '@playwright/test';
-import {test} from './utils_e2e.ts';
+import {test, login_user, login} from './utils_e2e.ts';
+import {accessibilityCheck} from './shared/accessibility.ts';
+
+test.beforeAll(async ({browser}, workerInfo) => {
+ await login_user(browser, workerInfo, 'user2');
+});
async function assertSelectedLines(page, nums) {
const pageAssertions = async () => {
@@ -75,3 +80,19 @@ test('Readable diff', async ({page}, workerInfo) => {
}
}
});
+
+test('Username highlighted in commits', async ({browser}, workerInfo) => {
+ const page = await login({browser}, workerInfo);
+ await page.goto('/user2/mentions-highlighted/commits/branch/main');
+ // check first commit
+ await page.getByRole('link', {name: 'A commit message which'}).click();
+ await expect(page.getByRole('link', {name: '@user2'})).toHaveCSS('background-color', /(.*)/);
+ await expect(page.getByRole('link', {name: '@user1'})).toHaveCSS('background-color', 'rgba(0, 0, 0, 0)');
+ await accessibilityCheck({page}, ['.commit-header'], [], []);
+ // check second commit
+ await page.goto('/user2/mentions-highlighted/commits/branch/main');
+ await page.locator('tbody').getByRole('link', {name: 'Another commit which mentions'}).click();
+ await expect(page.getByRole('link', {name: '@user2'})).toHaveCSS('background-color', /(.*)/);
+ await expect(page.getByRole('link', {name: '@user1'})).toHaveCSS('background-color', 'rgba(0, 0, 0, 0)');
+ await accessibilityCheck({page}, ['.commit-header'], [], []);
+});
From 019e38a7461c9cbf0d5086a9c019eb0df467786d Mon Sep 17 00:00:00 2001
From: Otto Richter
Date: Tue, 12 Nov 2024 17:20:36 +0100
Subject: [PATCH 4/4] chore(ci): Upload screenshots on test failure
---
.forgejo/workflows/testing.yml | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/.forgejo/workflows/testing.yml b/.forgejo/workflows/testing.yml
index 1d68ea1771..ffe51c5bec 100644
--- a/.forgejo/workflows/testing.yml
+++ b/.forgejo/workflows/testing.yml
@@ -121,6 +121,13 @@ jobs:
USE_REPO_TEST_DIR: 1
PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD: 1
CHANGED_FILES: ${{steps.changed-files.outputs.all_changed_files}}
+ - name: Upload screenshots on failure
+ if: failure()
+ uses: https://code.forgejo.org/forgejo/upload-artifact@v4
+ with:
+ name: screenshots.zip
+ path: /workspace/forgejo/forgejo/tests/e2e/test-artifacts/*/*.png
+ retention-days: 3
test-remote-cacher:
if: vars.ROLE == 'forgejo-coding' || vars.ROLE == 'forgejo-testing'
runs-on: docker