-
Notifications
You must be signed in to change notification settings - Fork 42
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
[IO-1335][internal] E2E setup and teardown #648
Conversation
IO-1335 E2E Setup task
Write a pytest setup function to setup and seed data beforehand with example datasets in place etc. No need to have a lot of data in place, mainly one or a few of each major data types:
Creation should run on whatever server is setup in the test config task. IO-1334 Ideally, do this with plain HTTP calls, so that if this fails, it's obviously a broken setup task, not darwin-py Acceptance Criteria
Note This is largely to be expanded on later, when actual tests are being written, so limit it to the above for now. The only variable should be whether the test team is created, so pros and cons in https://www.notion.so/v7labs/E2E-Testing-setup-a22f058e207447ae8dd0b9fa4d368e22 |
if not isinstance(session.config.cache, pytest.Cache): | ||
raise TypeError("Pytest caching is not enabled, but E2E tests require it") | ||
|
||
session.config.cache.set("server", server) | ||
session.config.cache.set("api_key", api_key) | ||
session.config.cache.set("team_slug", team_slug) | ||
|
||
global datasets |
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.
Normally a very bad practice, this is one of the few exceptions, as this function can only run once in scope. It's also the only way to solve this issue without storing the item in a custom format, and this is not pickleable.
if not isinstance(session.config.cache, pytest.Cache): | ||
raise TypeError("Pytest caching is not enabled, but E2E tests require it") | ||
|
||
global datasets |
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.
Normally a very bad practice, this is one of the few exceptions, as this function can only run once in scope. It's also the only way to solve this issue without storing the item in a custom format, and this is not pickleable.
datasets = setup_tests(ConfigValues(server=server, api_key=api_key, team_slug=team_slug)) | ||
|
||
print("Sleeping for 10 seconds to allow the server to catch up") | ||
sleep(10) |
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.
Sleep is also usually bad, but for test setup to account for latency, it's that or polling.
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.
So is the intendid use here for a test something along the lines of
def test_upload():
pytest_sessionstart()
run_workflow_I_want_to_test()
pytest_sessionfinish()
If so, perhaps should add it as a decorator that does startup and teardown
@e2e.basic
def test():
run_workflow_I_want_to_test()
Nah, TBH, Pytest is maybe not perfect for this kind of integration testing but it can be persuaded into it effectively, and given we are already familiar with it elsewhere, it seems to make better sense. |
Problem
Need to be able to run E2E tests with appropriate setup and teardown in place
Solution
Added setup, teardown, config, and data seeding functions.
Changelog
Added data seeding
Added Setup functions
Added teardown functions
Added pytest configuration functions
Added before and after run hooks