Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add initial playwright config #18442

Closed
wants to merge 16 commits into from

Conversation

kdumontnu
Copy link
Contributor

@kdumontnu kdumontnu commented Jan 29, 2022

Closes #18346

Test adding playwright support for end to end tests.

Current implementation:

  • use make test-e2e to run default e2e setup using sqlite
    • (re)builds gitea binary
    • uses integration test templates to build app.ini
    • runs gitea web and playwright tests
      • (not yet implemented) initialize/test DB before each test fixture
      • run series of assertion tests (eg. load page, assert load, click button...)
      • (not yet implemented) reset/cleanup database
  • can alternatively use make test-e2e-mysql or any of the other supported DBs in integration tests
  • can use test-e2e#test_name or test-e2e-mysql#test_name to run subset of tests

For example, I run as:

TEST_PGSQL_HOST=localhost:5432 TEST_PGSQL_DBNAME=gitea_e2e TEST_PGSQL_USERNAME=gitea TEST_PGSQL_PASSWORD=gitea TEST_PGSQL_SCHEMA='' NO_DEPS_PLAYWRIGHT=1 make test-e2e-pgsql

If you remove NO_DEPS_PLAYWRIGHT=1 it will run as root to install playwright + dependencies. Otherwise, you'll have to run npx playwright install --with-deps

You can uncomment the two lines in my test script to try the visual testing.

Some questions to answer:

How to handle DBs?

This is the last major hickup I have. Running in CI is easy because we start with a fresh DB every time. Running locally is a bit more difficult, because I want to put the DB back in the state that it started in at the end of the test. Ideas:

  • Is there a way to create a fresh DB using gitea? It looks like xorm supports creating tables, maybe I can write a wrapper for that?
  • Maybe I can expose or copy some of the functions in integration_test.go to prep and teardown the environment.
  • I can create a database "dump" at the beginning of the tests and restore at the end.

Can any makefile experts take a look at the makefile commands?

How to structure tests?

  • Since end-to-end tests should emulate a production environment, I think they should be able to run in parallel. That is any data that is created should not interfere with other tests (maybe there is a simple way to enforce this by prepending the test name hash to any emulated user data).
  • Currently I put the demo test in ./tools/e2e/tests/. Haven't investigated syncing with jest.

Browsers we want to test on?

Currently, I've enabled presets for chromium, firefox, webkit, Mobile Chrome, and Mobile Safari. Some explanation for Playwright browsers is here. All options for presets are here.

@kdumontnu kdumontnu force-pushed the kd/ci-playwright branch 6 times, most recently from 76b8808 to 63c5046 Compare January 29, 2022 02:00
.drone.yml Outdated
pull: always
image: gitea/test_env:linux-amd64 # https://gitea.com/gitea/test-env
commands:
- curl -sL https://deb.nodesource.com/setup_16.x | bash - && apt-get install -y nodejs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A little off topic, it's better for the owners to maintain a latest test_env to get nodejs , then we do not need to do curl again

.drone.yml Outdated Show resolved Hide resolved
- timeout -s ABRT 40m make test-e2e-pgsql
environment:
GOPROXY: https://goproxy.cn
TAGS: bindata
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this tag is for go build only in my mind.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just updated the summary. I'm actually building the binary before running e2e tests. Alternatively, I could throw an error "Build gitea before running e2e tests", but I like this better right now.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 29, 2022
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Jan 29, 2022

My opinions for the questions:

Should I set up a new database for 2e2 tests?

I think so, because other database might have been change a lot during other tests

Where do we want to save the e2e test files and how do we want to name them?
Currently I put the demo test in ./tools/e2e/tests/, but would we rather put .spec.js files along-side template and/or vue files?

For tests: maybe it's just like what Go testings, there are *_test.go along-side each go file, and there are also separate integration tests for multi-page tests.
For output result: S3/MinIO, or NFS, or something else, even SVN is a choice which is very friendly for binary data.

I'm starting off with an empty DB. We COULD populate a database with initial data-set or start with an empty server, populate the data using api and/or user interactions.

Agree

What browsers do we want to test on?

Do we have choices? playwright supports Chromium/Firefox/WebKit. Maybe one choice is Chromium only, another choice is Firefox+WebKit only (if they both work, Chrome should work).

Makefile Outdated
@@ -483,6 +484,26 @@ test-mssql-migration: migrations.mssql.test migrations.individual.mssql.test gen
GITEA_ROOT="$(CURDIR)" GITEA_CONF=integrations/mssql.ini ./migrations.mssql.test -test.failfast
GITEA_ROOT="$(CURDIR)" GITEA_CONF=integrations/mssql.ini ./migrations.individual.mssql.test -test.failfast

.PHONY: test-e2e
test-e2e: test-e2e-pgsql
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not depend on a specific database for the "offline" target, just assume the user has configured gitea appropriately. If you want psql on the CI, that's fine but it should be a separate target.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I'm not sure how how you mean. I updated the makefile to be more like the integration tests, so you can specify what DB type you want to use.

We need to pick a default database to write to for testing. If we use the developer's settings from app.ini I think we risk corrupting their development DB. I could create an e2e.ini.tmpl file and load that like we do for integration tests.

Makefile Outdated Show resolved Hide resolved
Makefile Outdated

.PHONY: test-e2e\#%
test-e2e\#%: test-e2e-pgsql\#%
echo "Why do I need this?"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are just because make does not support passing arguments to targets, but I think we should just document the appropriate npx jest <something> command to run single tests. I also wonder if we could just launch jest to run the actual tests, so we never need to npx playwright.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, it's because make... I wanted to keep it similar to the integration tests, but I can get rid of this if you really want.

@silverwind
Copy link
Member

Where do we want to save the e2e test files and how do we want to name them?

Can it be done in jest files, e.g. <file>.test.js?

What browsers do we want to test on?

Chrome and Firefox should be a good start.

Should I set up a new database for 2e2 tests?
I'm starting off with an empty DB. We COULD populate a database with initial data-set or start with an empty server, populate the data using api and/or user interactions.

If possible, I'd prefer it to work with an existing database without destroying anything existing, e.g. create a dedicated test repo and operate on that only. With jest, this can be done with proper setup and teardown code, but it'll be a bit of extra work.

.drone.yml Outdated Show resolved Hide resolved
@codecov-commenter

This comment was marked as off-topic.

@kdumontnu
Copy link
Contributor Author

My opinions for the questions:

Should I set up a new database for 2e2 tests?

I think so, because other database might have been change a lot during other tests

I would hope that the test DBs are cleaned up at the end of integration tests, but I don't fully understand that process (is that the purpose of USE_REPO_TEST_DIR?). We probably shouldn't support running integration tests and e2e tests simultaneously, so as long as both pieces of code clean up after they run (pass or fail) it should be okay to use the same data.

If possible, I'd prefer it to work with an existing database without destroying anything existing, e.g. create a dedicated test repo and operate on that only. With jest, this can be done with proper setup and teardown code, but it'll be a bit of extra work.

Hmm, I mostly meant hijacking on the integration test databases. I really don't want to be writing to users' databases in tests. Seems like bad news... Still I would like to to reset the database to a known state in the teardown.

Where do we want to save the e2e test files and how do we want to name them?
Currently I put the demo test in ./tools/e2e/tests/, but would we rather put .spec.js files along-side template and/or vue files?

For tests: maybe it's just like what Go testings, there are *_test.go along-side each go file, and there are also separate integration tests for multi-page tests. For output result: S3/MinIO, or NFS, or something else, even SVN is a choice which is very friendly for binary data.

Can it be done in jest files, e.g. .test.js?

We can name/put the test files anywhere we want. I need to check into integrating better with jest, but also they are testing different things, so not sure how closely we want to link them.

I'm starting off with an empty DB. We COULD populate a database with initial data-set or start with an empty server, populate the data using api and/or user interactions.

Agree

Ha, this was meant to be a question. Still trying to decide how we want to init the tests with data. I'll experiment with some options, but

What browsers do we want to test on?

Do we have choices? playwright supports Chromium/Firefox/WebKit. Maybe one choice is Chromium only, another choice is Firefox+WebKit only (if they both work, Chrome should work).

@wxiaoguang Full documentation here: https://playwright.dev/docs/browsers
Essentially, Chromium, Firefox, WebKit, but you can also specify viewport, touch, mobile and other parameters. One option is to specify from preset "devices" with prepopulated values. However, we can build our own tests. I've initially set up "Desktop Chrome", "Desktop Firefox", "Desktop Safari", "Pixel 5", "iPhone 12".

@kdumontnu kdumontnu force-pushed the kd/ci-playwright branch 8 times, most recently from 85afb14 to ee8c3e0 Compare February 12, 2022 04:48
@kdumontnu kdumontnu force-pushed the kd/ci-playwright branch 3 times, most recently from 7501672 to dbd5ab1 Compare February 20, 2022 00:44
@kdumontnu kdumontnu force-pushed the kd/ci-playwright branch 2 times, most recently from 3e77f2f to c27fd83 Compare June 22, 2022 21:20
@kdumontnu kdumontnu force-pushed the kd/ci-playwright branch 4 times, most recently from 77d2011 to 884ebe4 Compare June 23, 2022 00:27
@kdumontnu kdumontnu marked this pull request as ready for review June 23, 2022 03:31
@6543 6543 added this to the 1.18.0 milestone Jun 23, 2022

GiteaFlags=()

[[ -v GITEA_CUSTOM ]] && GiteaFlags+=(-C "${GITEA_CUSTOM}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The built-in bash on macOS is ancient. When I run make test-e2e, I get this:

./tools/e2e/run_e2e.sh: line 9: conditional binary operator expected
kill: usage: kill [-s sigspec | -n signum | -sigspec] pid | jobspec ... or kill -l [sigspec]
make: *** [test-e2e-sqlite] Error 1

It should either use this shebang:

#!/usr/bin/env bash

... or use more compatible syntax. The latter will probably lead to fewer gotchas.

test-e2e-sqlite: TAGS+=sqlite sqlite_unlock_notify
test-e2e-sqlite: build generate-ini-sqlite
npx playwright install $(PLAYWRIGHT_FLAGS)
GITEA_ROOT=$(CURDIR) GITEA_URL="http://localhost:3003" GITEA_EXECUTABLE=$(EXECUTABLE) GITEA_CONF=integrations/sqlite.ini ./tools/e2e/run_e2e.sh
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to override the GITEA_URL? This port is wrong when I run the make task.

@kdumontnu kdumontnu mentioned this pull request Jun 24, 2022
7 tasks
@kdumontnu
Copy link
Contributor Author

Closed by #20123

@kdumontnu kdumontnu closed this Sep 2, 2022
@lunny lunny removed this from the 1.18.0 milestone Sep 3, 2022
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. type/testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI: Front-End Test Framework
9 participants