-
-
Notifications
You must be signed in to change notification settings - Fork 315
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
E2E Integration test environment setup #746
Changes from 13 commits
f11f10f
226c4a6
668e67c
88488a0
71817c0
07cd5b8
007f883
608e452
bb6b7f0
3382926
11bf8fa
deb7f46
5efcfe0
edd7854
94e7827
2cb167b
063cb7a
12ef48d
e772b0a
4c67223
954901c
49eb096
a710cfc
2a07f16
64352b8
649e904
f26ca02
6395b19
ff3e5cf
62439b2
b7657c3
ba894a2
04b970b
609dd49
1db88d1
cfa53d9
e5bfc70
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
name: Run E2E integration tests | ||
on: [push, pull_request] | ||
|
||
jobs: | ||
build: | ||
runs-on: ubuntu-latest | ||
# We only want to run on external PRs, since internal PRs are covered by "push" | ||
# This prevents this from running twice on internal PRs | ||
if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name != github.repository | ||
steps: | ||
- uses: actions/checkout@v2 | ||
|
||
# In this step, this action saves a list of existing images, | ||
# the cache is created without them in the post run. | ||
# It also restores the cache if it exists. | ||
- uses: satackey/action-docker-layer-caching@v0.0.11 | ||
# Ignore the failure of a step and avoid terminating the job. | ||
continue-on-error: true | ||
with: | ||
key: mathesar-docker-cache-integ-tests-{hash} | ||
restore-keys: | | ||
mathesar-docker-cache-integ-tests- | ||
|
||
- name: Copy env file | ||
run: cp .env.example .env | ||
|
||
# The code is checked out under uid 1001 - reset this to 1000 for the | ||
# container to run tests successfully | ||
- name: Fix permissions | ||
run: sudo chown -R 1000:1000 . | ||
|
||
- name: Build the stack | ||
run: docker-compose up --build -d | ||
|
||
- name: Sleep for 60 seconds | ||
run: sleep 60s | ||
shell: bash | ||
|
||
# TODO: This needs to be handled inside the tests | ||
- name: Run migrations | ||
run: docker exec mathesar_service python manage.py migrate | ||
|
||
- name: Run type installation | ||
run: docker exec mathesar_service python install.py --skip-confirm | ||
|
||
- name: Run integ tests with pytest | ||
run: docker exec mathesar_service pytest --browser chromium --browser webkit --browser firefox tests/ | ||
|
||
- uses: actions/upload-artifact@v2 | ||
if: ${{ failure() || success() }} | ||
with: | ||
name: Recordings | ||
path: videos/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,4 +33,4 @@ jobs: | |
run: docker-compose up --build -d | ||
|
||
- name: Run tests with pytest | ||
run: docker exec mathesar_service pytest | ||
run: docker exec mathesar_service pytest mathesar/ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've added it. Thanks. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ env = | |
DEBUG=False | ||
SECRET_KEY=hdC7qKjaFXNBjJ4heMMlOMrP-6j1-OvZpPf87DAXyaw | ||
DJANGO_DATABASE_KEY=default | ||
DJANGO_DATABASE_URL=postgres://mathesar:mathesar@db:5432/mathesar_django | ||
MATHESAR_DATABASES=(mathesar_db_test|postgres://mathesar:mathesar@db:5432/mathesar_db_test) | ||
DJANGO_DATABASE_URL=postgres://mathesar:mathesar@mathesar_db:5432/mathesar_django | ||
MATHESAR_DATABASES=(mathesar_db_test|postgres://mathesar:mathesar@mathesar_db:5432/mathesar_db_test) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the usual way is for the hostname to be the header of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was |
||
TEST=True | ||
MODE=DEVELOPMENT |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
def test_base_page(browser): | ||
context = browser.new_context(record_video_dir='videos/') | ||
page = context.new_page() | ||
page.goto('http://localhost:8000') | ||
assert page.inner_text('h1') == 'Welcome to Mathesar!' | ||
context.close() |
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 think we should use the timestamp in the name of the artifact.
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 may want to upload an artifact in a job and download in another job, time stamping the name would make it hard for us to do that.
I do understand the concern where multiple workflows could overwrite the artifact. So, I've modified the action to use different paths for each workflow in ba894a2. Let me know if this looks good.
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.
Looks good.