Updates E2E Testing Documentation for local development #1542
Conversation
ndegwamartin
force-pushed
the
e2e-local-tests
branch
from
571b635 to
e84d99d
Compare
ndegwamartin
force-pushed
the
e2e-local-tests
branch
from
e84d99d to
5dcd8eb
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1542 +/- ##
============================================
+ Coverage 46.61% 46.63% +0.01%
- Complexity 678 679 +1
============================================
Files 90 90
Lines 5886 5886
Branches 834 834
============================================
+ Hits 2744 2745 +1
Misses 2827 2827
+ Partials 315 314 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
bashir2
left a comment
bashir2
left a comment
There was a problem hiding this comment.
Thanks for creating this useful doc @ndegwamartin .
| -e OUTPUT_PARQUET_VIEW_PATH=/workspace/e2e-tests/FHIR_SEARCH_HAPI/VIEWS_TIMESTAMP_1 \ | ||
| -e SINK_PATH=http://localhost:9001/fhir \ | ||
| -e SINK_USERNAME=hapi -e SINK_PASSWORD=hapi \ | ||
| -v $(pwd):/workspace \ |
There was a problem hiding this comment.
Consider adding a note about output directories under this path that might be overwritten.
There was a problem hiding this comment.
I think this would mainly be the path under the OUTPUT_PARQUET_VIEW_PATH env, correct? I've added a note.
There was a problem hiding this comment.
I think in this case, it is PARQUET_PATH too, see here.
doc/run-cloud-build-locally.md
Outdated
| ${_REPOSITORY}/e2e-tests:${_TAG} | ||
| ``` | ||
|
|
||
| or just the script directly: |
There was a problem hiding this comment.
Again, since this is a local environment, I would recommend to people to use the non-docker version as running a local script is easier to debug. That said, I would also make a note that in the Cloud Build env this is done through docker (and keep the sample docker run).
There was a problem hiding this comment.
This is correct, the direct script run is documented as an alternative immediately after this.
Updated the documentation to reflect the suggestion above.
There was a problem hiding this comment.
As a side note, my original understanding was that we needed to fully replicate the previously documented cloud build workflow run via:
cloud-build-local --dryrun=false .
There was a problem hiding this comment.
Should we also consider renaming the file(and instructions title) from run-cloud-build-locally.md ?
There was a problem hiding this comment.
This document that you have created is actually better than cloud-build-local --dryrun=false . because even if we could use cloud-build-local we still needed to replicate the steps locally to be able to debug (which is what this doc is). Please feel free if you want to use a better name for this, e.g., debug-cloud-build-locally.md.
doc/run-cloud-build-locally.md
Outdated
| ### 4. Bring up controller and Spark containers | ||
|
|
||
| ```bash | ||
| PIPELINE_CONFIG=/workspace/docker/config DWH_ROOT=/workspace/e2e-tests/controller-spark/dwh docker compose -f docker/compose-controller-spark-sql-single.yaml up --force-recreate -d |
There was a problem hiding this comment.
Don't you need a volume mapping for /workspace?
nit: Also please break the long line (didn't our default formatter catch this?).
There was a problem hiding this comment.
The bind mount is defined in the compose config file here -
fhir-data-pipes/docker/compose-controller-spark-sql-single.yaml
Lines 60 to 62 in 547612d
There was a problem hiding this comment.
Don't think our default formatter(Spotless) can reformat these commands as appears in the .md files. We probably need to break these down manually.
doc/run-cloud-build-locally.md
Outdated
| ${_REPOSITORY}/e2e-tests:${_TAG} | ||
| ``` | ||
|
|
||
| or just the script directly: |
There was a problem hiding this comment.
This document that you have created is actually better than cloud-build-local --dryrun=false . because even if we could use cloud-build-local we still needed to replicate the steps locally to be able to debug (which is what this doc is). Please feel free if you want to use a better name for this, e.g., debug-cloud-build-locally.md.
| -e OUTPUT_PARQUET_VIEW_PATH=/workspace/e2e-tests/FHIR_SEARCH_HAPI/VIEWS_TIMESTAMP_1 \ | ||
| -e SINK_PATH=http://localhost:9001/fhir \ | ||
| -e SINK_USERNAME=hapi -e SINK_PASSWORD=hapi \ | ||
| -v $(pwd):/workspace \ |
There was a problem hiding this comment.
I think in this case, it is PARQUET_PATH too, see here.
3 participants
Description of what I changed
Resolves #1534
E2E test
TESTED:
Please replace this with a description of how you tested your PR beyond the
automated e2e/unit tests.
Checklist: I completed these to help reviewers :)
I have read and will follow the
review process.
I am familiar with Google Style Guides for the language I have coded in.
No? Please take some time and review
Java and
Python style guides.
My IDE is configured to follow the Google
code styles.
No? Unsure? ->
configure your IDE.
I have added tests to cover my changes. (If you refactored existing
code that was well tested you do not have to add tests)
I ran
mvn clean packageright before creating this pull request andadded all formatting changes to my commit.
If I made any Python code changes, I ran
black .,pylint .andpyright. right before creating this pull request and added allformatting changes to my commit.
All new and existing tests passed.
My pull request is based on the latest changes of the master branch.
No? Unsure? -> execute command
git pull --rebase upstream master