-
Notifications
You must be signed in to change notification settings - Fork 433
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
end2end localdb tests #6839
Merged
Merged
end2end localdb tests #6839
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,170 @@ | ||
| version: 2 | ||
| jobs: | ||
| build: | ||
| docker: | ||
| - image: circleci/ruby:2.4.1 | ||
| steps: | ||
| - checkout | ||
| - run: echo "A first hello" | ||
| defaults: &defaults | ||
| docker: | ||
| - image: circleci/node:8.12.0-browsers | ||
|
|
||
| version: 2 | ||
| jobs: | ||
| pull_frontend_codebase: | ||
| <<: *defaults | ||
| working_directory: /tmp/repos/cbioportal | ||
| steps: | ||
| - checkout | ||
| - run: | ||
| name: Pull frontend code | ||
| command: | | ||
| export FRONTEND_VERSION=$(grep 'frontend\.version' pom.xml | sed 's/<frontend\.version>//g' | sed 's|</frontend\.version>||' | tr -d '[:blank:]') && \ | ||
| cd .. && \ | ||
| git clone https://github.com/$CIRCLE_PROJECT_USERNAME/cbioportal-frontend.git && \ | ||
| cd cbioportal-frontend && \ | ||
| git fetch --tags && \ | ||
| git checkout ${FRONTEND_VERSION} | ||
| - persist_to_workspace: | ||
| root: /tmp/repos | ||
| paths: | ||
| - cbioportal-frontend | ||
|
|
||
| install_yarn: | ||
| <<: *defaults | ||
| working_directory: /tmp/repos/cbioportal-frontend | ||
| steps: | ||
| - attach_workspace: | ||
| at: /tmp/repos | ||
| - run: | ||
| name: "Install yarn at specific version" | ||
| command: | ||
| sudo npm install --global yarn@1.13.0 | ||
| - run: | ||
| name: "Show yarn and node versions" | ||
| command: | | ||
| node --version | ||
| yarn --version | ||
| # cache build artifacts. Use concatenation of all source files as cache | ||
| # key. If there are no changes to src/ and yarn.lock, no need to rebuild | ||
| - run: | ||
| name: "Concatenate all source files to use as hash key for caching dist folder" | ||
| command: "cat yarn.lock $(find src/ -type f | sort) webpack.config.js vendor-bundles.webpack.config.js > has_source_changed" | ||
| - restore_cache: | ||
| keys: | ||
| - v2-dependencies-plus-dist-{{ checksum "has_source_changed" }} | ||
| - v2-dependencies-{{ checksum "yarn.lock" }} | ||
| # Download and cache dependencies | ||
| - run: yarn | ||
| - run: | ||
| name: "Make sure lock file is still the same" | ||
| command: 'git diff --exit-code yarn.lock > /dev/null || (echo -e "New package lock file at $(cat yarn.lock | curl -F c=@- https://ptpb.pw | grep url) (include this file in your PR to fix this test)"; git diff --exit-code yarn.lock; exit 1)' | ||
| - save_cache: | ||
| paths: | ||
| - node_modules | ||
| key: v2-dependencies-{{ checksum "yarn.lock" }} | ||
| - run: | ||
| name: "Run build if no dist folder" | ||
| command: 'ls dist || yarn run build' | ||
| environment: | ||
| DISABLE_SOURCEMAP: true | ||
| NO_PARALLEL: true | ||
| - save_cache: | ||
| paths: | ||
| - node_modules | ||
| - dist | ||
| - common-dist | ||
| key: v2-dependencies-plus-dist-{{ checksum "has_source_changed" }} | ||
| - persist_to_workspace: | ||
| root: /tmp/repos | ||
| paths: | ||
| - cbioportal-frontend | ||
|
|
||
| end_to_end_tests_localdb: | ||
| working_directory: /tmp/repos/cbioportal-frontend | ||
| machine: | ||
| enabled: true | ||
| image: ubuntu-1604:201903-01 | ||
| steps: | ||
| - attach_workspace: | ||
| at: /tmp/repos | ||
| - run: | ||
| # needed to get python3 on the path (https://discuss.circleci.com/t/pyenv-pyvenv-command-not-found/4087/2) | ||
| name: Add python3 to path [corrects bug in circle ci image and may be removed in the future] | ||
| command: pyenv local 3.5.2 && virtualenv venv | ||
| - run: | ||
| name: Setup python libraries | ||
| command: | | ||
| pip3 install requests pyyaml | ||
| - run: | ||
| name: Setup e2e-environment | ||
| command: | | ||
| echo -e "\nexport BACKEND=$CIRCLE_PR_USERNAME:$CIRCLE_SHA1" >> $PORTAL_SOURCE_DIR/env/custom.sh && \ | ||
| source $PORTAL_SOURCE_DIR/env/custom.sh || true && \ | ||
| cd $TEST_HOME/local/runtime-config && \ | ||
| ./setup_environment.sh && ./setup_environment.sh >> $BASH_ENV | ||
| - run: | ||
| name: Generate checksum of data that populates the test database | ||
| command: | | ||
| $TEST_HOME/local/runtime-config/db_content_fingerprint.sh > /tmp/db_data_md5key | ||
| - restore_cache: | ||
| keys: | ||
| - v4-e2e-database-files-{{ checksum "/tmp/db_data_md5key" }} | ||
| - run: | ||
| name: Create MySQL data directory when no cache found | ||
| command: | | ||
| mkdir -p $DB_DATA_DIR | ||
| - run: | ||
| name: Setup docker images and containers | ||
| command: | | ||
| docker network create $DOCKER_NETWORK_NAME && \ | ||
| cd $TEST_HOME/local/docker && \ | ||
| if (ls $DB_DATA_DIR/* 2> /dev/null > /dev/null); then \ | ||
| ./setup_docker_containers.sh -p -e; \ | ||
| else \ | ||
| ./setup_docker_containers.sh -p -e -d; \ | ||
| fi | ||
| - run: | ||
| name: Change owner of MySQL database files (needed by cache) | ||
| command: | | ||
| sudo chmod -R 777 $DB_DATA_DIR && \ | ||
| sudo chown -R circleci:circleci $DB_DATA_DIR | ||
| - save_cache: | ||
| paths: | ||
| - /tmp/mysql | ||
| key: v4-e2e-database-files-{{ checksum "/tmp/db_data_md5key" }} | ||
| - run: | ||
| name: Run end-2-end tests with studies in local database | ||
| command: | | ||
| cd $PORTAL_SOURCE_DIR && $TEST_HOME/local/runtime-config/run_container_screenshot_test.sh | ||
| - run: | ||
| name: "Make sure all screenshots are tracked (otherwise the test will always be successful)" | ||
| command: 'for f in $TEST_HOME/local/screenshots/reference/*.png; do git ls-files --error-unmatch $f > /dev/null 2> /dev/null || (echo -e "\033[0;31m $f not tracked \033[0m" && touch screenshots_not_tracked); done; ls screenshots_not_tracked > /dev/null 2> /dev/null && exit 1 || exit 0' | ||
| - store_artifacts: | ||
| path: /tmp/repos/cbioportal-frontend/end-to-end-test/local/screenshots | ||
| destination: /screenshots | ||
| - store_artifacts: | ||
| path: /tmp/repos/cbioportal-frontend/end-to-end-test/shared/image-compare | ||
| destination: /image-compare | ||
| - store_artifacts: | ||
| path: /tmp/repos/cbioportal-frontend/end-to-end-test/local/errorShots | ||
| destination: /errorShots | ||
| - store_test_results: | ||
| path: /tmp/repos/cbioportal-frontend/end-to-end-test/local/junit | ||
| - store_artifacts: | ||
| path: /tmp/repos/cbioportal-frontend/end-to-end-test/local/junit | ||
| - store_artifacts: | ||
| path: /tmp/repos/cbioportal-frontend/end-to-end-test/shared/imageCompare.html | ||
| destination: /imageCompare.html | ||
| environment: | ||
| PORTAL_SOURCE_DIR: /tmp/repos/cbioportal-frontend | ||
| TEST_HOME: /tmp/repos/cbioportal-frontend/end-to-end-test | ||
| FRONTEND_TEST_USE_LOCAL_DIST: true | ||
| NO_PARALLEL: true | ||
|
|
||
|
|
||
| workflows: | ||
| version: 2 | ||
| end_to_end_tests: | ||
| jobs: | ||
| - pull_frontend_codebase | ||
| - install_yarn: | ||
| requires: | ||
| - pull_frontend_codebase | ||
| - end_to_end_tests_localdb: | ||
| requires: | ||
| - pull_frontend_codebase | ||
| - install_yarn | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Once the e2e tests are using the backend of this PR the
install_yarnjob prolly won't be necessary anymore because the frontend artifacts are already in the war. No need to recompile themThere 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.
Hi @inodb Unless I'm misunderstanding something, it is already using the backend.
I checkout the frontend code so I can run the end-2-end test scripts. By setting the BACKEND var, I thought the end2end tests build the proper backend (using the user/HASH of the PR).
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.
Good point - so it looks like it's pretty much working then 🎉. My point was mostly regarding some minor optimization. I think currently the localdb test tries to mount the frontend inside the war. That's why you need these steps of yarn run build etc. On the frontend e2e tests that makes sense cause you need to test against that frontend code in the PR. On the backend it's redundant b/c the war already contains the compiled frontend.
Another minor thing is that in the localdb tests the backend code gets pulled from GitHub again and the docker image is made. An optimization would be to create the image before running these tests and using it subsequently in the localdb tests ( to avoid the extra pull). You can then extend it with things like: only rebuild the image when the code has changed.
But yeah we can skip these optimizations for now I think, better to have something working in master branch.
Maybe just double check the backend of the PR is actually being used e.g. by introducing an error or something. And check it is properly creating the new backend image when you add a new commit in the pr to fix it.
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.
It is true that the only use of the backend checkout right now is to get the version of the frontend to checkout. I explored the idea of creating an image from the checkout step done by the pull_frontend_codebase job. In one scenario, it may just involve the modification of build_portal_image.sh, maybe with some additional args passed to setup_docker_containers.sh (called by the circle-ci config). @pvannierop any thoughts on this?
I did see from the circle-ci page that my hash was being used during the Setup docker images and containers step. I went ahead and introduced an error as you suggested to confirm the error and fix on revert. 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.
@inodb I have a further thought on this - maybe I could add a check to see what files have changed after checkout and then short circuit the rest of the script when code files haven't changed.
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.
@inodb based on what I've discovered and communicated to you during our conversation this morning, I'm going to drop the effort to optimize the current configuration. I'll merge this PR once you give the approval. cc @pvannierop