From f2a23c962a1e3fee28b0083f2e24e640361b0780 Mon Sep 17 00:00:00 2001 From: Otto Richter Date: Tue, 1 Oct 2024 17:45:38 +0200 Subject: [PATCH 1/4] ci: merge e2e workflow into testing.yml ci: cache frontend build across jobs ci: ensure caches are saved with zstd work around https://github.com/actions/cache/issues/1169 ci: require unit tests for remote cacher - prevents unnecessary runs in case the unit tests already fail - starts the integration tests about 2 minutes earlier - should give some overall speedup to the CI run, because the long integration tests are run and finish earlier, and the cacher tests should still usually finish in time - does not save any computing resources, just provides quicker results when runners are not under high load --- .../build-backend/action.yaml | 2 +- .../workflows-composite/setup-env/action.yaml | 3 +- .forgejo/workflows/e2e.yml | 37 ------------------ .forgejo/workflows/testing.yml | 38 ++++++++++++++++++- 4 files changed, 40 insertions(+), 40 deletions(-) delete mode 100644 .forgejo/workflows/e2e.yml diff --git a/.forgejo/workflows-composite/build-backend/action.yaml b/.forgejo/workflows-composite/build-backend/action.yaml index 193ff911e1..ada372b834 100644 --- a/.forgejo/workflows-composite/build-backend/action.yaml +++ b/.forgejo/workflows-composite/build-backend/action.yaml @@ -6,7 +6,7 @@ runs: - uses: actions/cache@v4 id: cache-backend with: - path: '/workspace/forgejo/forgejo/gitea' + path: ${{github.workspace}}/gitea key: backend-build-${{ github.sha }} - if: steps.cache-backend.outputs.cache-hit != 'true' run: | diff --git a/.forgejo/workflows-composite/setup-env/action.yaml b/.forgejo/workflows-composite/setup-env/action.yaml index 65ffa8341c..d40ee52d20 100644 --- a/.forgejo/workflows-composite/setup-env/action.yaml +++ b/.forgejo/workflows-composite/setup-env/action.yaml @@ -4,7 +4,8 @@ runs: - name: setup user and permissions run: | git config --add safe.directory '*' - adduser --quiet --comment forgejo --disabled-password forgejo + # ignore if the user already exists (like with the playwright image) + adduser --quiet --comment forgejo --disabled-password forgejo || true chown -R forgejo:forgejo . - uses: https://codeberg.org/fnetx/setup-cache-go@b2214eaf6fb44c7e8512c0f462a2c3ec31f86a73 with: diff --git a/.forgejo/workflows/e2e.yml b/.forgejo/workflows/e2e.yml deleted file mode 100644 index 7e27765513..0000000000 --- a/.forgejo/workflows/e2e.yml +++ /dev/null @@ -1,37 +0,0 @@ -name: e2e - -on: - pull_request: - paths: - - Makefile - - playwright.config.js - - .forgejo/workflows/e2e.yml - - tests/e2e/** - - web_src/js/** - - web_src/css/form.css - - templates/webhook/shared-settings.tmpl - - templates/org/team/new.tmpl - -jobs: - test-e2e: - if: ${{ !startsWith(vars.ROLE, 'forgejo-') }} - runs-on: docker - container: - image: 'code.forgejo.org/oci/playwright:latest' - steps: - - uses: https://code.forgejo.org/actions/checkout@v4 - - uses: https://code.forgejo.org/actions/setup-go@v5 - with: - go-version-file: "go.mod" - - run: | - git config --add safe.directory '*' - 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 - env: - USE_REPO_TEST_DIR: 1 diff --git a/.forgejo/workflows/testing.yml b/.forgejo/workflows/testing.yml index 960f3d716e..f50b948bdf 100644 --- a/.forgejo/workflows/testing.yml +++ b/.forgejo/workflows/testing.yml @@ -36,6 +36,17 @@ jobs: - run: make checks-frontend - run: make test-frontend-coverage - run: make frontend + - name: Install zstd for cache saving + # works around https://github.com/actions/cache/issues/1169, because the + # consuming job has zstd and doesn't restore the cache otherwise + run: | + apt-get update -qq + apt-get -q install -qq -y zstd + - name: "Cache frontend build for playwright testing" + uses: actions/cache/save@v4 + with: + path: ${{github.workspace}}/public/assets + key: frontend-build-${{ github.sha }} test-unit: if: ${{ !startsWith(vars.ROLE, 'forgejo-') }} runs-on: docker @@ -75,10 +86,35 @@ jobs: RACE_ENABLED: 'true' TAGS: bindata TEST_ELASTICSEARCH_URL: http://elasticsearch:9200 - test-remote-cacher: + test-e2e: if: ${{ !startsWith(vars.ROLE, 'forgejo-') }} runs-on: docker needs: [backend-checks, frontend-checks] + container: + image: 'code.forgejo.org/oci/playwright:latest' + steps: + - uses: https://code.forgejo.org/actions/checkout@v4 + - uses: ./.forgejo/workflows-composite/setup-env + - name: "Restore frontend build" + uses: actions/cache/restore@v4 + id: cache-frontend + with: + path: ${{github.workspace}}/public/assets + key: frontend-build-${{ github.sha }} + - name: "Build frontend (if not cached)" + if: steps.cache-frontend.outputs.cache-hit != 'true' + run: | + su forgejo -c 'make deps-frontend frontend' + - uses: ./.forgejo/workflows-composite/build-backend + - run: | + su forgejo -c 'make generate test-e2e-sqlite' + timeout-minutes: 40 + env: + USE_REPO_TEST_DIR: 1 + test-remote-cacher: + if: ${{ !startsWith(vars.ROLE, 'forgejo-') }} + runs-on: docker + needs: [backend-checks, frontend-checks, test-unit] container: image: 'code.forgejo.org/oci/node:20-bookworm' strategy: From 7765153b405e7c519350bfb05d3f85f70643f7fa Mon Sep 17 00:00:00 2001 From: Otto Richter Date: Wed, 11 Sep 2024 22:34:33 +0200 Subject: [PATCH 2/4] tests(e2e): Allow tests to run only on file changes - supports glob patterns in testfiles - only runs tests on changes - always runs tests without specified patterns tests(e2e): refactor global watch patterns tests(e2e): add watch patterns to test files --- tests/e2e/actions.test.e2e.js | 12 ++ tests/e2e/changes.go | 114 ++++++++++++++++++ tests/e2e/dashboard-ci-status.test.e2e.js | 5 + tests/e2e/e2e_test.go | 6 + tests/e2e/example.test.e2e.js | 7 ++ tests/e2e/explore.test.e2e.js | 6 + tests/e2e/issue-comment.test.e2e.js | 7 ++ tests/e2e/issue-sidebar.test.e2e.js | 7 ++ tests/e2e/markdown-editor.test.e2e.js | 6 + tests/e2e/markup.test.e2e.js | 5 + tests/e2e/org-settings.test.e2e.js | 7 ++ tests/e2e/profile_actions.test.e2e.js | 7 ++ tests/e2e/reaction-selectors.test.e2e.js | 6 + tests/e2e/release.test.e2e.js | 11 ++ tests/e2e/repo-code.test.e2e.js | 14 +-- ...st.e2e.js => repo-commitgraph.test.e2e.js} | 14 +++ tests/e2e/repo-migrate.test.e2e.js | 5 + tests/e2e/repo-settings.test.e2e.js | 9 ++ tests/e2e/repo-wiki.test.e2e.js | 6 + tests/e2e/right-settings-button.test.e2e.js | 7 ++ tests/e2e/webauthn.test.e2e.js | 6 + 21 files changed, 260 insertions(+), 7 deletions(-) create mode 100644 tests/e2e/changes.go rename tests/e2e/{commit-graph-branch-selector.test.e2e.js => repo-commitgraph.test.e2e.js} (65%) diff --git a/tests/e2e/actions.test.e2e.js b/tests/e2e/actions.test.e2e.js index b049a93ed9..01ddb7b971 100644 --- a/tests/e2e/actions.test.e2e.js +++ b/tests/e2e/actions.test.e2e.js @@ -1,4 +1,16 @@ // @ts-check + +// @watch start +// templates/repo/actions/** +// web_src/css/actions.css +// web_src/js/components/ActionRunStatus.vue +// web_src/js/components/RepoActionView.vue +// modules/actions/** +// modules/structs/workflow.go +// routers/api/v1/repo/action.go +// routers/web/repo/actions/** +// @watch end + import {expect} from '@playwright/test'; import {test, login_user, load_logged_in_context} from './utils_e2e.js'; diff --git a/tests/e2e/changes.go b/tests/e2e/changes.go new file mode 100644 index 0000000000..acc1a796a4 --- /dev/null +++ b/tests/e2e/changes.go @@ -0,0 +1,114 @@ +// Copyright 2024 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package e2e + +import ( + "bufio" + "os" + "strings" + + "code.gitea.io/gitea/modules/log" + + "github.com/gobwas/glob" +) + +var ( + changesetFiles []string + changesetAvailable bool + globalFullRun bool +) + +func initChangedFiles() { + var changes string + changes, changesetAvailable = os.LookupEnv("CHANGED_FILES") + // the output of the Action seems to actually contain \n and not a newline literal + changesetFiles = strings.Split(changes, `\n`) + log.Info("Only running tests covered by a subset of test files. Received the following list of CHANGED_FILES: %q", changesetFiles) + + globalPatterns := []string{ + // meta and config + "Makefile", + "playwright.config.js", + ".forgejo/workflows/testing.yml", + "tests/e2e/*.go", + "tests/e2e/shared/*", + // frontend files + "frontend/*.js", + "frontend/{base,index}.css", + // templates + "templates/base/**", + } + fullRunPatterns := []glob.Glob{} + for _, expr := range globalPatterns { + fullRunPatterns = append(fullRunPatterns, glob.MustCompile(expr, '.', '/')) + } + globalFullRun = false + for _, changedFile := range changesetFiles { + for _, pattern := range fullRunPatterns { + if pattern.Match(changedFile) { + globalFullRun = true + log.Info("Changed files match global test pattern, running all tests") + return + } + } + } +} + +func canSkipTest(testFile string) bool { + // run all tests when environment variable is not set or changes match global pattern + if !changesetAvailable || globalFullRun { + return false + } + + for _, changedFile := range changesetFiles { + if strings.HasSuffix(testFile, changedFile) { + return false + } + for _, pattern := range getWatchPatterns(testFile) { + if pattern.Match(changedFile) { + return false + } + } + } + return true +} + +func getWatchPatterns(filename string) []glob.Glob { + file, err := os.Open(filename) + if err != nil { + log.Fatal(err.Error()) + } + defer file.Close() + scanner := bufio.NewScanner(file) + + watchSection := false + patterns := []glob.Glob{} + for scanner.Scan() { + line := scanner.Text() + // check for watch block + if strings.HasPrefix(line, "// @watch") { + if watchSection { + break + } + watchSection = true + } + if !watchSection { + continue + } + + line = strings.TrimPrefix(line, "// ") + if line != "" { + globPattern, err := glob.Compile(line, '.', '/') + if err != nil { + log.Fatal("Invalid glob pattern '%s' (skipped): %v", line, err) + } + patterns = append(patterns, globPattern) + } + } + // if no watch block in file + if !watchSection { + patterns = append(patterns, glob.MustCompile("*")) + } + return patterns +} diff --git a/tests/e2e/dashboard-ci-status.test.e2e.js b/tests/e2e/dashboard-ci-status.test.e2e.js index 1ff68b6334..ec61bfac76 100644 --- a/tests/e2e/dashboard-ci-status.test.e2e.js +++ b/tests/e2e/dashboard-ci-status.test.e2e.js @@ -1,4 +1,9 @@ // @ts-check + +// @watch start +// web_src/js/components/DashboardRepoList.vue +// @watch end + import {expect} from '@playwright/test'; import {test, login_user, load_logged_in_context} from './utils_e2e.js'; diff --git a/tests/e2e/e2e_test.go b/tests/e2e/e2e_test.go index 44a6897bf4..3dc8bfd00c 100644 --- a/tests/e2e/e2e_test.go +++ b/tests/e2e/e2e_test.go @@ -38,6 +38,7 @@ func TestMain(m *testing.M) { defer cancel() tests.InitTest(true) + initChangedFiles() testE2eWebRoutes = routers.NormalRoutes() os.Unsetenv("GIT_AUTHOR_NAME") @@ -100,6 +101,11 @@ func TestE2e(t *testing.T) { _, filename := filepath.Split(path) testname := filename[:len(filename)-len(filepath.Ext(path))] + if canSkipTest(path) { + fmt.Printf("No related changes for test, skipping: %s\n", filename) + continue + } + t.Run(testname, func(t *testing.T) { // Default 2 minute timeout onForgejoRun(t, func(*testing.T, *url.URL) { diff --git a/tests/e2e/example.test.e2e.js b/tests/e2e/example.test.e2e.js index 86abdf685e..c163c8bb42 100644 --- a/tests/e2e/example.test.e2e.js +++ b/tests/e2e/example.test.e2e.js @@ -1,4 +1,11 @@ // @ts-check + +// @watch start +// templates/user/auth/** +// web_src/js/features/user-** +// modules/{user,auth}/** +// @watch end + import {expect} from '@playwright/test'; import {test, login_user, save_visual} from './utils_e2e.js'; diff --git a/tests/e2e/explore.test.e2e.js b/tests/e2e/explore.test.e2e.js index 9603443b35..b64eca78e3 100644 --- a/tests/e2e/explore.test.e2e.js +++ b/tests/e2e/explore.test.e2e.js @@ -1,6 +1,12 @@ // @ts-check // document is a global in evaluate, so it's safe to ignore here // eslint playwright/no-conditional-in-test: 0 + +// @watch start +// templates/explore/** +// web_src/modules/fomantic/** +// @watch end + import {expect} from '@playwright/test'; import {test} from './utils_e2e.js'; diff --git a/tests/e2e/issue-comment.test.e2e.js b/tests/e2e/issue-comment.test.e2e.js index ee2e3a4c89..55dccf1ebd 100644 --- a/tests/e2e/issue-comment.test.e2e.js +++ b/tests/e2e/issue-comment.test.e2e.js @@ -1,4 +1,11 @@ // @ts-check + +// @watch start +// web_src/js/features/comp/** +// web_src/js/features/repo-** +// templates/repo/issue/view_content/* +// @watch end + import {expect} from '@playwright/test'; import {test, login_user, login} from './utils_e2e.js'; diff --git a/tests/e2e/issue-sidebar.test.e2e.js b/tests/e2e/issue-sidebar.test.e2e.js index 0311910a34..f9e8380fd4 100644 --- a/tests/e2e/issue-sidebar.test.e2e.js +++ b/tests/e2e/issue-sidebar.test.e2e.js @@ -1,4 +1,11 @@ // @ts-check + +// @watch start +// templates/repo/issue/view_content/** +// web_src/css/repo/issue-** +// web_src/js/features/repo-issue** +// @watch end + import {expect} from '@playwright/test'; import {test, login_user, login} from './utils_e2e.js'; diff --git a/tests/e2e/markdown-editor.test.e2e.js b/tests/e2e/markdown-editor.test.e2e.js index 4a3b414b10..a1f2a2d96c 100644 --- a/tests/e2e/markdown-editor.test.e2e.js +++ b/tests/e2e/markdown-editor.test.e2e.js @@ -1,4 +1,10 @@ // @ts-check + +// @watch start +// web_src/js/features/comp/ComboMarkdownEditor.js +// web_src/css/editor/combomarkdowneditor.css +// @watch end + import {expect} from '@playwright/test'; import {test, load_logged_in_context, login_user} from './utils_e2e.js'; diff --git a/tests/e2e/markup.test.e2e.js b/tests/e2e/markup.test.e2e.js index 920537d08f..a2b795e852 100644 --- a/tests/e2e/markup.test.e2e.js +++ b/tests/e2e/markup.test.e2e.js @@ -1,4 +1,9 @@ // @ts-check + +// @watch start +// web_src/css/markup/** +// @watch end + import {expect} from '@playwright/test'; import {test} from './utils_e2e.js'; diff --git a/tests/e2e/org-settings.test.e2e.js b/tests/e2e/org-settings.test.e2e.js index 5ff0975a26..2a0fe69608 100644 --- a/tests/e2e/org-settings.test.e2e.js +++ b/tests/e2e/org-settings.test.e2e.js @@ -1,4 +1,11 @@ // @ts-check + +// @watch start +// templates/org/team/new.tmpl +// web_src/css/form.css +// web_src/js/features/org-team.js +// @watch end + import {expect} from '@playwright/test'; import {test, login_user, login} from './utils_e2e.js'; import {validate_form} from './shared/forms.js'; diff --git a/tests/e2e/profile_actions.test.e2e.js b/tests/e2e/profile_actions.test.e2e.js index dcec0cd83c..d168037041 100644 --- a/tests/e2e/profile_actions.test.e2e.js +++ b/tests/e2e/profile_actions.test.e2e.js @@ -1,4 +1,11 @@ // @ts-check + +// @watch start +// routers/web/user/** +// templates/shared/user/** +// web_src/js/features/common-global.js +// @watch end + import {expect} from '@playwright/test'; import {test, login_user, load_logged_in_context} from './utils_e2e.js'; diff --git a/tests/e2e/reaction-selectors.test.e2e.js b/tests/e2e/reaction-selectors.test.e2e.js index 2a9c62bb4d..5e0ea5b519 100644 --- a/tests/e2e/reaction-selectors.test.e2e.js +++ b/tests/e2e/reaction-selectors.test.e2e.js @@ -1,4 +1,10 @@ // @ts-check + +// @watch start +// web_src/js/features/comp/ReactionSelector.js +// routers/web/repo/issue.go +// @watch end + import {expect} from '@playwright/test'; import {test, login_user, load_logged_in_context} from './utils_e2e.js'; diff --git a/tests/e2e/release.test.e2e.js b/tests/e2e/release.test.e2e.js index ac1e101f98..4ae4f31883 100644 --- a/tests/e2e/release.test.e2e.js +++ b/tests/e2e/release.test.e2e.js @@ -1,4 +1,15 @@ // @ts-check + +// @watch start +// models/repo/attachment.go +// modules/structs/attachment.go +// routers/web/repo/** +// services/attachment/** +// services/release/** +// templates/repo/release/** +// web_src/js/features/repo-release.js +// @watch end + import {expect} from '@playwright/test'; import {test, login_user, save_visual, load_logged_in_context} from './utils_e2e.js'; import {validate_form} from './shared/forms.js'; diff --git a/tests/e2e/repo-code.test.e2e.js b/tests/e2e/repo-code.test.e2e.js index 62c4f557c1..9d9653d2fe 100644 --- a/tests/e2e/repo-code.test.e2e.js +++ b/tests/e2e/repo-code.test.e2e.js @@ -1,4 +1,11 @@ // @ts-check + +// @watch start +// web_src/js/features/repo-code.js +// web_src/css/repo.css +// services/gitdiff/** +// @watch end + import {expect} from '@playwright/test'; import {test, login_user, load_logged_in_context} from './utils_e2e.js'; @@ -77,10 +84,3 @@ test('Readable diff', async ({page}, workerInfo) => { } } }); - -test('Commit graph overflow', async ({page}) => { - await page.goto('/user2/diff-test/graph'); - await expect(page.getByRole('button', {name: 'Mono'})).toBeInViewport({ratio: 1}); - await expect(page.getByRole('button', {name: 'Color'})).toBeInViewport({ratio: 1}); - await expect(page.locator('.selection.search.dropdown')).toBeInViewport({ratio: 1}); -}); diff --git a/tests/e2e/commit-graph-branch-selector.test.e2e.js b/tests/e2e/repo-commitgraph.test.e2e.js similarity index 65% rename from tests/e2e/commit-graph-branch-selector.test.e2e.js rename to tests/e2e/repo-commitgraph.test.e2e.js index db849320b9..7bb3e1f23f 100644 --- a/tests/e2e/commit-graph-branch-selector.test.e2e.js +++ b/tests/e2e/repo-commitgraph.test.e2e.js @@ -1,4 +1,11 @@ // @ts-check + +// @watch start +// templates/repo/graph.tmpl +// web_src/css/features/gitgraph.css +// web_src/js/features/repo-graph.js +// @watch end + import {expect} from '@playwright/test'; import {test, login_user, load_logged_in_context} from './utils_e2e.js'; @@ -6,6 +13,13 @@ test.beforeAll(async ({browser}, workerInfo) => { await login_user(browser, workerInfo, 'user2'); }); +test('Commit graph overflow', async ({page}) => { + await page.goto('/user2/diff-test/graph'); + await expect(page.getByRole('button', {name: 'Mono'})).toBeInViewport({ratio: 1}); + await expect(page.getByRole('button', {name: 'Color'})).toBeInViewport({ratio: 1}); + await expect(page.locator('.selection.search.dropdown')).toBeInViewport({ratio: 1}); +}); + test('Switch branch', async ({browser}, workerInfo) => { const context = await load_logged_in_context(browser, workerInfo, 'user2'); const page = await context.newPage(); diff --git a/tests/e2e/repo-migrate.test.e2e.js b/tests/e2e/repo-migrate.test.e2e.js index 63328e0900..7a9fc08fb2 100644 --- a/tests/e2e/repo-migrate.test.e2e.js +++ b/tests/e2e/repo-migrate.test.e2e.js @@ -1,4 +1,9 @@ // @ts-check + +// @watch start +// web_src/js/features/repo-migrate.js +// @watch end + import {expect} from '@playwright/test'; import {test, login_user, load_logged_in_context} from './utils_e2e.js'; diff --git a/tests/e2e/repo-settings.test.e2e.js b/tests/e2e/repo-settings.test.e2e.js index b7b0884b26..f215016c15 100644 --- a/tests/e2e/repo-settings.test.e2e.js +++ b/tests/e2e/repo-settings.test.e2e.js @@ -1,4 +1,13 @@ // @ts-check + +// @watch start +// templates/webhook/shared-settings.tmpl +// templates/repo/settings/** +// web_src/css/{form,repo}.css +// web_src/css/modules/grid.css +// web_src/js/features/comp/WebHookEditor.js +// @watch end + import {expect} from '@playwright/test'; import {test, login_user, login} from './utils_e2e.js'; import {validate_form} from './shared/forms.js'; diff --git a/tests/e2e/repo-wiki.test.e2e.js b/tests/e2e/repo-wiki.test.e2e.js index 4599fbd0c7..eb6c033748 100644 --- a/tests/e2e/repo-wiki.test.e2e.js +++ b/tests/e2e/repo-wiki.test.e2e.js @@ -1,4 +1,10 @@ // @ts-check + +// @watch start +// templates/repo/wiki/** +// web_src/css/repo** +// @watch end + import {expect} from '@playwright/test'; import {test} from './utils_e2e.js'; diff --git a/tests/e2e/right-settings-button.test.e2e.js b/tests/e2e/right-settings-button.test.e2e.js index 4f2b09b4c1..87e10c040c 100644 --- a/tests/e2e/right-settings-button.test.e2e.js +++ b/tests/e2e/right-settings-button.test.e2e.js @@ -1,4 +1,11 @@ // @ts-check + +// @watch start +// templates/org/** +// templates/repo/** +// web_src/js/webcomponents/overflow-menu.js +// @watch end + import {expect} from '@playwright/test'; import {test, login_user, load_logged_in_context} from './utils_e2e.js'; diff --git a/tests/e2e/webauthn.test.e2e.js b/tests/e2e/webauthn.test.e2e.js index e11c17c331..f98cd47126 100644 --- a/tests/e2e/webauthn.test.e2e.js +++ b/tests/e2e/webauthn.test.e2e.js @@ -2,6 +2,12 @@ // SPDX-License-Identifier: MIT // @ts-check +// @watch start +// templates/user/auth/** +// templates/user/settings/** +// web_src/js/features/user-** +// @watch end + import {expect} from '@playwright/test'; import {test, login_user, load_logged_in_context} from './utils_e2e.js'; From 72749bcf70c4c22371fbb0d8fd0cf46a2e688b95 Mon Sep 17 00:00:00 2001 From: Otto Richter Date: Tue, 1 Oct 2024 23:34:22 +0200 Subject: [PATCH 3/4] ci: (always|only) run e2e tests based on changes - detect changed files for the run - let e2e files specify which related files they "watch" - only run e2e tests based on pattern matching or when generic files change - fallback to full runs if env not specified --- .forgejo/workflows/testing.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.forgejo/workflows/testing.yml b/.forgejo/workflows/testing.yml index f50b948bdf..55ac4454b5 100644 --- a/.forgejo/workflows/testing.yml +++ b/.forgejo/workflows/testing.yml @@ -94,6 +94,8 @@ jobs: image: 'code.forgejo.org/oci/playwright:latest' steps: - uses: https://code.forgejo.org/actions/checkout@v4 + with: + fetch-depth: 20 - uses: ./.forgejo/workflows-composite/setup-env - name: "Restore frontend build" uses: actions/cache/restore@v4 @@ -106,11 +108,17 @@ jobs: run: | su forgejo -c 'make deps-frontend frontend' - uses: ./.forgejo/workflows-composite/build-backend + - name: Get changed files + id: changed-files + uses: https://code.forgejo.org/fossdd/changed-files@v45 + with: + separator: '\n' - run: | su forgejo -c 'make generate test-e2e-sqlite' timeout-minutes: 40 env: USE_REPO_TEST_DIR: 1 + CHANGED_FILES: ${{steps.changed-files.outputs.all_changed_files}} test-remote-cacher: if: ${{ !startsWith(vars.ROLE, 'forgejo-') }} runs-on: docker From 88f653b58377f3a112323b335dd2dfd6c8476272 Mon Sep 17 00:00:00 2001 From: Otto Richter Date: Thu, 3 Oct 2024 20:29:36 +0200 Subject: [PATCH 4/4] docs(e2e): Update e2e test instructions for changed file patterns chore: let fnetx review e2e tests --- CODEOWNERS | 3 +++ tests/e2e/README.md | 57 +++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/CODEOWNERS b/CODEOWNERS index d46efc052b..6ca34a69df 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -16,6 +16,9 @@ templates/.* @caesar @crystal @gusted ## the issue sidebar was touched by fnetx templates/repo/issue/view_content/sidebar.* @fnetx +# Playwright tests +tests/e2e/.* @fnetx + # Files related to Go development. # The modules usually don't require much knowledge about Forgejo and could diff --git a/tests/e2e/README.md b/tests/e2e/README.md index 65520115e6..36b9a187c3 100644 --- a/tests/e2e/README.md +++ b/tests/e2e/README.md @@ -110,9 +110,13 @@ If you have a [forgejo runner](https://code.forgejo.org/forgejo/runner/), you can use it to run the test jobs: ``` -forgejo-runner exec -W .forgejo/workflows/e2e.yml --event=pull_request +forgejo-runner exec -W .forgejo/workflows/testing.yml -j test-e2e ``` +Note that the CI workflow has some logic to run tests based on changed files only. +This might conflict with your local setup and not run all the desired tests +because it might only look at file changes in your latest commit. + ### Run e2e tests with another database This approach is not currently used, @@ -212,9 +216,52 @@ Take a look at `shared/forms.js` and some other places for inspiration. ### List related files coverage -If you think your playwright tests covers an important aspect of some template, CSS or backend files, -consider adding the paths to `.forgejo/workflows/e2e.yml` in the path filter. +To speed up the CI pipelines and avoid running expensive tests too often, +only a selection of tests is run by default, +based on the changed files. -It ensures that future modifications to this file will be tested as well. +At the top of each playwright test file, +list the files or file patterns that are covered by your test. +Often, these are files that you modified for your feature or bugfix, +or that you looked at (and might still have open in your IDE), +because your fix depends on their behaviour. -Currently, we do not run the e2e tests on all changes. +#### Which files to watch? + +The set of files your test "watches" depends on the kind of test you write. +If you only test for the presence of an element and do no accessibility or placement checks, +you won't detect broken visual appearance and there is little reason to watch CSS files. + +However, if your test also checks that an element is correctly positioned +(e.g. that it does not overflow the page), +or has accessibiltiy properties (includes colour contrast), +also list stylesheets that define the behaviour your test depends on. + +Watching the place that generate the selectors you use +(typically templates, but can also be JavaScript) +is a must, to ensure that someone modifying the markup notices that your selectors fail +(e.g. because an id or class was renamed). + +If you are unsure about the exact set of files, +feel free to ask other contributors. + +#### How to specify the patterns? + +You put filenames and patterns as blocks between two `// @watch` comments. +An example that watches changes on (in order) +a single file, +a full recursive subfolder, +two files with a shorthand pattern, +and a set of files with a certain ending: + +~~~ +// @watch start +// templates/webhook/shared-settings.tmpl +// templates/repo/settings/** +// web_src/css/{form,repo}.css +// web_src/css/modules/*.css +// @watch end +~~~ + +The patterns are evaluated on a "first-match" basis. +Under the hood, [gobwas/glob](https://github.com/gobwas/glob) is used.