-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Conversation
76b8808
to
63c5046
Compare
.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 |
There was a problem hiding this comment.
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
- timeout -s ABRT 40m make test-e2e-pgsql | ||
environment: | ||
GOPROXY: https://goproxy.cn | ||
TAGS: bindata |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
My opinions for the questions:
I think so, because other database might have been change a lot during other tests
For tests: maybe it's just like what Go testings, there are
Agree
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
|
||
.PHONY: test-e2e\#% | ||
test-e2e\#%: test-e2e-pgsql\#% | ||
echo "Why do I need this?" |
There was a problem hiding this comment.
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
.
There was a problem hiding this comment.
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.
Can it be done in jest files, e.g.
Chrome and Firefox should be a good start.
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 |
5576e8f
to
34fe239
Compare
This comment was marked as off-topic.
This comment was marked as off-topic.
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
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.
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.
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
@wxiaoguang Full documentation here: https://playwright.dev/docs/browsers |
85afb14
to
ee8c3e0
Compare
7501672
to
dbd5ab1
Compare
3e77f2f
to
c27fd83
Compare
77d2011
to
884ebe4
Compare
884ebe4
to
b08bfb5
Compare
007c199
to
0236676
Compare
|
||
GiteaFlags=() | ||
|
||
[[ -v GITEA_CUSTOM ]] && GiteaFlags+=(-C "${GITEA_CUSTOM}") |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
Closed by #20123 |
Closes #18346
Test adding playwright support for end to end tests.
Current implementation:
make test-e2e
to run default e2e setup using sqlitegitea web
and playwright testsmake test-e2e-mysql
or any of the other supported DBs in integration teststest-e2e#test_name
ortest-e2e-mysql#test_name
to run subset of testsFor example, I run as:
If you remove
NO_DEPS_PLAYWRIGHT=1
it will run as root to install playwright + dependencies. Otherwise, you'll have to runnpx 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:
integration_test.go
to prep and teardown the environment.Can any makefile experts take a look at the makefile commands?
How to structure tests?
./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
, andMobile Safari
. Some explanation for Playwright browsers is here. All options for presets are here.