test: add foundation for e2e testing using docker #41
Conversation
ryanchristo
force-pushed
the
ryan/22-docker-and-test
branch
from
d950437
to
3bc5608
Compare
ryanchristo
force-pushed
the
ryan/22-docker-and-test
branch
from
3bc5608
to
bc832cb
Compare
.github/workflows/test.yml
Outdated
| on: | ||
| push: | ||
| # branches: | ||
| # - main | ||
| pull_request: | ||
| # branches: | ||
| # - main |
There was a problem hiding this comment.
We can leave this as is for now or re-enable for pushes to main and pull requests against main before merging.
There was a problem hiding this comment.
I think a good start for this would to have it run on pull requests against main before merging.
Even thought you said it takes a few minutes for these to run, it can and will provide good value.
And I'm sure that's something that can be sped up eventually.
There was a problem hiding this comment.
Sorry, my comment was a bit confusing. Currently with these lines commented out, it will run for every push and every pull request. I had to do this in order for the tests to run in this pull request (before being merged into main). The alternative would be to limit these to running only on push to main and pull request against main.
There was a problem hiding this comment.
Either way, uncommented for now.
There was a problem hiding this comment.
Also, looks like tests are running on this pull request even after uncommenting and not sure why I needed to comment out the first time. Maybe a momentary github actions issue.
| # TODO(#42): executor result should be success after successful execution | ||
|
|
||
| # check proposal executor result | ||
| #if ! psql "$DATABASE_URL" -c "SELECT * FROM proposals WHERE proposal_id=$proposal_id AND executor_result=PROPOSAL_EXECUTOR_RESULT_SUCCESS;"; then | ||
| # echo "indexed proposal with PROPOSAL_EXECUTOR_RESULT_SUCCESS not found" | ||
| # exit 1 | ||
| #fi |
ryanchristo
changed the title
| RUN pip install load_dotenv | ||
| RUN pip install psycopg2 | ||
| RUN pip install sentry_sdk | ||
| RUN pip install tenacity |
There was a problem hiding this comment.
Lines 15-18 shouldn't be necessary, poetry install should pick these up since we included them as dependencies via poetry as a package manager
There was a problem hiding this comment.
i.e. https://github.com/regen-network/indexer/blob/main/poetry.lock#L258
poetry.lock and poetry install are basically like package.lock and npm install in the python world
There was a problem hiding this comment.
Thanks! Yea, for some reason I was hitting errors that these were not installed but that was awhile back and seems to be working fine without these lines now. Not sure why that was an issue previously.
There was a problem hiding this comment.
Ah! Nvm, still hitting the same errors when starting the containers:
ModuleNotFoundError: No module named 'dotenv'
There was a problem hiding this comment.
I'm going to leave them for now but maybe there is something we can do to prevent needing them in the future.
There was a problem hiding this comment.
LGTM, left some small comments and also tACK (i tried this locally and worked for me!)
Only other thing i can think is maybe you can add these instructions into a readme.
We've needed a readme in the repo for a while, maybe for now you could just create a basic template for the readme, add these instructions, and then the rest of the readme gets filled out at later time
WDYT?
Yea, I can add a template with instructions. Good idea. |
Ref: #16
This pull request sets the foundation for continuous integration tests for the indexer (#16) and was used to test #22. A followup pull request is still needed for #16 to be resolved but this provides some visibility that would be valuable in the meantime.
How to test
View test logs for github actions:
Also, if you want to use for testing locally:
Build docker containers (this takes some time but only needs to be run once or after updating docker files):
Run docker containers (and continue running non-tester containers after test scripts):
Run docker containers (and stop all containers after test scripts):
Stop and remove containers (clean up previous run before running again):