From 35f481fde2919b985057ebb18c28a23e69316168 Mon Sep 17 00:00:00 2001 From: Trent Smith <1429913+Bento007@users.noreply.github.com> Date: Tue, 10 Jan 2023 16:34:29 -0800 Subject: [PATCH 1/5] fix(coverage): adding code coverage (#3894) - add code coverage for python unit tests - run coverage for every PR and upload report to Codecov - add make recipes to generate coverage report - add allure test report for unit tests - update coverage version --- .github/workflows/push-tests.yml | 50 ++++++++++++++++++++++++++++++-- Makefile | 31 +++++++++++++++----- requirements.txt | 2 +- 3 files changed, 73 insertions(+), 10 deletions(-) diff --git a/.github/workflows/push-tests.yml b/.github/workflows/push-tests.yml index 8f461dbb95aa2..e5e038cbb8340 100644 --- a/.github/workflows/push-tests.yml +++ b/.github/workflows/push-tests.yml @@ -275,7 +275,6 @@ jobs: SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK }} if: failure() && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/staging' || github.ref == 'refs/heads/prod') - backend-wmg-unit-test: runs-on: ubuntu-20.04 steps: @@ -324,7 +323,6 @@ jobs: SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK }} if: failure() && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/staging' || github.ref == 'refs/heads/prod') - processing-unit-test: runs-on: ubuntu-20.04 steps: @@ -369,6 +367,12 @@ jobs: path: /home/runner/work/single-cell-data-portal/single-cell-data-portal/allure-results retention-days: 20 + - uses: actions/upload-artifact@v3 + with: + name: coverage + path: /home/runner/work/single-cell-data-portal/single-cell-data-portal/.coverage* + retention-days: 3 + - uses: 8398a7/action-slack@v3 with: status: ${{ job.status }} @@ -480,10 +484,52 @@ jobs: SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK }} if: failure() && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/staging' || github.ref == 'refs/heads/prod') + submit-codecoverage: + needs: + - backend-unit-test + - backend-wmg-unit-test + - wmg-processing-unit-test + - processing-unit-test + runs-on: ubuntu-20.04 + steps: + - name: Configure AWS Credentials + uses: aws-actions/configure-aws-credentials@v1 + with: + aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY }} + aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }} + aws-region: us-west-2 + role-to-assume: ${{ secrets.AWS_ROLE_TO_ASSUME }} + role-duration-seconds: 1800 + - name: Login to ECR + uses: docker/login-action@v1 + with: + registry: ${{ secrets.ECR_REPO }} + - uses: actions/checkout@v2 + with: + fetch-depth: 0 + - uses: actions/download-artifact@v3 + with: + name: coverage + path: . + - name: coverage report + run: | + echo "DOCKER_REPO=${DOCKER_REPO}" > .env.ecr + make coverage/report-xml + - name: Upload coverage to Codecov + uses: codecov/codecov-action@v3 + with: + token: ${{ secrets.CODECOV_TOKEN }} + env_vars: OS,PYTHON + files: ./coverage.xml + flags: unittests + name: codecov-umbrella + publish-test-report: needs: - backend-unit-test - backend-wmg-unit-test + - wmg-processing-unit-test + - processing-unit-test - e2e-test runs-on: ubuntu-20.04 steps: diff --git a/Makefile b/Makefile index 28435699d762b..05bab6dede63e 100644 --- a/Makefile +++ b/Makefile @@ -8,7 +8,9 @@ ifeq ($(AWS_ACCESS_KEY_ID),) export TEST_AWS_PROFILE ?= single-cell-dev endif export DEPLOYMENT_STAGE ?= test - +COVERAGE_DATA_FILE=.coverage.$(shell git rev-parse --short HEAD) +COVERAGE_FILTER=--omit=*/dist-packages/*,*/backend/database/*,*/backend/scripts/*,*/site-packages/* --include=*/backend/*,*/tests/unit/* +export COVERAGE_RUN_ARGS:=--data-file=$(COVERAGE_DATA_FILE) --parallel-mode $(COVERAGE_FILTER) .PHONY: fmt fmt: @@ -26,8 +28,8 @@ unit-test: local-unit-test .PHONY: wmg-processing-unittest wmg-processing-unittest: # This target is intended to be run INSIDE the wmg processing container - DEPLOYMENT_STAGE=test PYTHONWARNINGS=ignore:ResourceWarning pytest tests/unit/wmg_processing \ - --rootdir=. --alluredir=./allure-results --verbose; + DEPLOYMENT_STAGE=test PYTHONWARNINGS=ignore:ResourceWarning coverage run $(COVERAGE_RUN_ARGS) -m pytest \ + tests/unit/wmg_processing/ --rootdir=. --alluredir=./allure-results --verbose; .PHONY: functional-test functional-test: local-functional-test @@ -143,15 +145,18 @@ local-shell: ## Open a command shell in one of the dev containers. ex: make loca docker-compose exec $(CONTAINER) bash .PHONY: local-unit-test -local-unit-test: local-unit-test-backend local-unit-test-wmg-processing# Run all backend and processing unit tests in the dev environment, with code coverage +local-unit-test: local-unit-test-backend local-unit-test-wmg-backend local-unit-test-wmg-processing local-unit-test-processing +# Run all backend and processing unit tests in the dev environment, with code coverage .PHONY: local-unit-test-backend local-unit-test-backend: - docker-compose run --rm -T backend bash -c "cd /single-cell-data-portal && python3 -m pytest tests/unit/backend/layers/"; + docker-compose run --rm -T backend bash -c \ + "cd /single-cell-data-portal && coverage run $(COVERAGE_RUN_ARGS) -m pytest --alluredir=./allure-results tests/unit/backend/layers/"; .PHONY: local-unit-test-wmg-backend local-unit-test-wmg-backend: - docker-compose run --rm -T backend bash -c "cd /single-cell-data-portal && python3 -m pytest tests/unit/backend/wmg/"; + docker-compose run --rm -T backend bash -c \ + "cd /single-cell-data-portal && coverage run $(COVERAGE_RUN_ARGS) -m pytest --alluredir=./allure-results tests/unit/backend/wmg/"; .PHONY: local-integration-test-backend local-integration-test-backend: @@ -160,7 +165,7 @@ local-integration-test-backend: .PHONY: local-unit-test-processing local-unit-test-processing: # Run processing-unittest target in `processing` Docker container docker-compose $(COMPOSE_OPTS) run --rm -e DEV_MODE_COOKIES= -T processing \ - bash -c "cd /single-cell-data-portal && python3 -m pytest tests/unit/processing/"; + bash -c "cd /single-cell-data-portal && coverage run $(COVERAGE_RUN_ARGS) -m pytest --alluredir=./allure-results tests/unit/processing/"; .PHONY: local-unit-test-wmg-processing local-unit-test-wmg-processing: # Run processing-unittest target in `wmg_processing` Docker container @@ -218,3 +223,15 @@ local-uploadsuccess: .env.ecr ## Run the upload success lambda with a dataset id .PHONY: local-cxguser-cookie local-cxguser-cookie: ## Get cxguser-cookie docker-compose $(COMPOSE_OPTS) run --rm backend bash -c "cd /single-cell-data-portal && python login.py" + +.PHONY: coverage/combine +coverage/combine: + - docker-compose $(COMPOSE_OPTS) run --rm -T backend bash -c "cd /single-cell-data-portal && coverage combine --data-file=$(COVERAGE_DATA_FILE)" + +.PHONY: coverage/report +coverage/report-xml: coverage/combine + docker-compose $(COMPOSE_OPTS) run --rm -T backend bash -c "cd /single-cell-data-portal && coverage xml --data-file=$(COVERAGE_DATA_FILE) -i --skip-empty" + +.PHONY: coverage/report +coverage/report-html: coverage/combine + docker-compose $(COMPOSE_OPTS) run --rm -T backend bash -c "cd /single-cell-data-portal && coverage html --data-file=$(COVERAGE_DATA_FILE) -i --skip-empty" diff --git a/requirements.txt b/requirements.txt index c25e0389092c5..f1ff1e28bf4b1 100644 --- a/requirements.txt +++ b/requirements.txt @@ -5,7 +5,7 @@ black==22.3.0 # Must be kept in sync with black version in .pre-commit-config.y boto3>=1.11.17 botocore>=1.14.17 click==8.1.3 -coverage>=2.0.15 +coverage>=6.5.0 flake8-black==0.3.2 # Must be kept in sync with flake8 version in .pre-commit-config.yaml flake8==4.0.1 # Must be kept in sync with flake8 version in .pre-commit-config.yaml furl From 3ae19ef48ee90bf12bab173b6f7a7e5302a532df Mon Sep 17 00:00:00 2001 From: Emanuele Bezzi Date: Wed, 11 Jan 2023 09:33:38 -0500 Subject: [PATCH 2/5] fix: curation API differentiates unpublished collections for revisions --- backend/portal/api/curation/curation-api.yml | 4 +++- .../v1/curation/collections/common.py | 21 +++++++++++++++---- .../backend/layers/api/test_curation_api.py | 2 +- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/backend/portal/api/curation/curation-api.yml b/backend/portal/api/curation/curation-api.yml index d262e6143d5f7..aba8dd49b9e55 100644 --- a/backend/portal/api/curation/curation-api.yml +++ b/backend/portal/api/curation/curation-api.yml @@ -582,7 +582,9 @@ components: description: Collection metadata properties: collection_url: - description: The CELLxGENE Discover URL for the Collection. + description: | + The CELLxGENE Discover URL for the Collection. This points to the canonical link unless it's a revision, + in which case it points to the unpublished revision version. type: string contact_email: $ref: "#/components/schemas/contact_email" diff --git a/backend/portal/api/curation/v1/curation/collections/common.py b/backend/portal/api/curation/v1/curation/collections/common.py index 385a3416b962d..e9e35b1856363 100644 --- a/backend/portal/api/curation/v1/curation/collections/common.py +++ b/backend/portal/api/curation/v1/curation/collections/common.py @@ -69,6 +69,7 @@ def reshape_for_curation_api( if is_published: # Published collection_id = collection_version.collection_id + collection_url = f"{get_collections_base_url()}/collections/{collection_id.id}" revision_of = None if not user_info.is_user_owner_or_allowed(collection_version.owner): _revising_in = None @@ -78,9 +79,21 @@ def reshape_for_curation_api( ) revising_in = _revising_in.version_id.id if _revising_in else None else: - # Unpublished - collection_id = collection_version.version_id - revision_of = collection_version.collection_id.id + # Unpublished - need to determine if it's a revision or first time collection + # For that, we look at whether the canonical collection is published + is_revision = collection_version.canonical_collection.originally_published_at is not None + if is_revision: + # If it's a revision, both collection_id and collection_url need to point to the version_id + collection_id = collection_version.version_id + collection_url = f"{get_collections_base_url()}/collections/{collection_id.id}" + revision_of = collection_version.collection_id.id + else: + # If it's an unpublished, unrevised collection, then collection_url will point to the permalink + # (aka the link to the canonical_id) and the collection_id will point to version_id. + # Also, revision_of should be None + collection_id = collection_version.version_id + collection_url = f"{get_collections_base_url()}/collections/{collection_version.collection_id}" + revision_of = None revising_in = None # get collection dataset attributes @@ -94,7 +107,7 @@ def reshape_for_curation_api( else: revised_at = None response = dict( - collection_url=f"{get_collections_base_url()}/collections/{collection_id.id}", + collection_url=collection_url, contact_email=collection_version.metadata.contact_email, contact_name=collection_version.metadata.contact_name, created_at=collection_version.created_at, diff --git a/tests/unit/backend/layers/api/test_curation_api.py b/tests/unit/backend/layers/api/test_curation_api.py index c6064283c1b62..d3e77ab98a631 100644 --- a/tests/unit/backend/layers/api/test_curation_api.py +++ b/tests/unit/backend/layers/api/test_curation_api.py @@ -353,7 +353,7 @@ def test__get_collections_with_auth__OK_6(self): resp = self._test_response(visibility="PRIVATE", auth=True) self.assertEqual(1, len(resp)) resp_collection = resp[0] - self.assertEqual(unpublished_collection_id.collection_id.id, resp_collection["revision_of"]) + self.assertIsNone(resp_collection["revision_of"]) def test__get_collections_no_auth_visibility_private__403(self): self._test_response(visibility="PRIVATE", status_code=403) From 4c31c62ff498ce57ba3a7ba3fa5eefdb3113c509 Mon Sep 17 00:00:00 2001 From: nayib-jose-gloria Date: Wed, 11 Jan 2023 10:59:54 -0500 Subject: [PATCH 3/5] remove unused var Signed-off-by: nayib-jose-gloria --- tests/unit/backend/layers/api/test_curation_api.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/backend/layers/api/test_curation_api.py b/tests/unit/backend/layers/api/test_curation_api.py index d3e77ab98a631..0e0017db3f440 100644 --- a/tests/unit/backend/layers/api/test_curation_api.py +++ b/tests/unit/backend/layers/api/test_curation_api.py @@ -349,7 +349,6 @@ def test__get_collections_with_auth__OK_5(self): def test__get_collections_with_auth__OK_6(self): "revision_of contains None if the collection is unpublished" - unpublished_collection_id = self.generate_unpublished_collection() resp = self._test_response(visibility="PRIVATE", auth=True) self.assertEqual(1, len(resp)) resp_collection = resp[0] From 0eadad1a0c0a5e05171d4a6090495581f2507385 Mon Sep 17 00:00:00 2001 From: nayib-jose-gloria Date: Wed, 11 Jan 2023 11:04:55 -0500 Subject: [PATCH 4/5] fix test Signed-off-by: nayib-jose-gloria --- tests/unit/backend/layers/api/test_curation_api.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/backend/layers/api/test_curation_api.py b/tests/unit/backend/layers/api/test_curation_api.py index 0e0017db3f440..3468d7bebe72f 100644 --- a/tests/unit/backend/layers/api/test_curation_api.py +++ b/tests/unit/backend/layers/api/test_curation_api.py @@ -349,6 +349,7 @@ def test__get_collections_with_auth__OK_5(self): def test__get_collections_with_auth__OK_6(self): "revision_of contains None if the collection is unpublished" + self.generate_unpublished_collection() resp = self._test_response(visibility="PRIVATE", auth=True) self.assertEqual(1, len(resp)) resp_collection = resp[0] From a7262c1870074ab34a378614b675935b730421e4 Mon Sep 17 00:00:00 2001 From: nayib-jose-gloria Date: Wed, 11 Jan 2023 11:11:03 -0500 Subject: [PATCH 5/5] Revert "fix(coverage): adding code coverage (#3894)" This reverts commit 35f481fde2919b985057ebb18c28a23e69316168. --- .github/workflows/push-tests.yml | 50 ++------------------------------ Makefile | 31 +++++--------------- requirements.txt | 2 +- 3 files changed, 10 insertions(+), 73 deletions(-) diff --git a/.github/workflows/push-tests.yml b/.github/workflows/push-tests.yml index e5e038cbb8340..8f461dbb95aa2 100644 --- a/.github/workflows/push-tests.yml +++ b/.github/workflows/push-tests.yml @@ -275,6 +275,7 @@ jobs: SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK }} if: failure() && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/staging' || github.ref == 'refs/heads/prod') + backend-wmg-unit-test: runs-on: ubuntu-20.04 steps: @@ -323,6 +324,7 @@ jobs: SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK }} if: failure() && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/staging' || github.ref == 'refs/heads/prod') + processing-unit-test: runs-on: ubuntu-20.04 steps: @@ -367,12 +369,6 @@ jobs: path: /home/runner/work/single-cell-data-portal/single-cell-data-portal/allure-results retention-days: 20 - - uses: actions/upload-artifact@v3 - with: - name: coverage - path: /home/runner/work/single-cell-data-portal/single-cell-data-portal/.coverage* - retention-days: 3 - - uses: 8398a7/action-slack@v3 with: status: ${{ job.status }} @@ -484,52 +480,10 @@ jobs: SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK }} if: failure() && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/staging' || github.ref == 'refs/heads/prod') - submit-codecoverage: - needs: - - backend-unit-test - - backend-wmg-unit-test - - wmg-processing-unit-test - - processing-unit-test - runs-on: ubuntu-20.04 - steps: - - name: Configure AWS Credentials - uses: aws-actions/configure-aws-credentials@v1 - with: - aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY }} - aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }} - aws-region: us-west-2 - role-to-assume: ${{ secrets.AWS_ROLE_TO_ASSUME }} - role-duration-seconds: 1800 - - name: Login to ECR - uses: docker/login-action@v1 - with: - registry: ${{ secrets.ECR_REPO }} - - uses: actions/checkout@v2 - with: - fetch-depth: 0 - - uses: actions/download-artifact@v3 - with: - name: coverage - path: . - - name: coverage report - run: | - echo "DOCKER_REPO=${DOCKER_REPO}" > .env.ecr - make coverage/report-xml - - name: Upload coverage to Codecov - uses: codecov/codecov-action@v3 - with: - token: ${{ secrets.CODECOV_TOKEN }} - env_vars: OS,PYTHON - files: ./coverage.xml - flags: unittests - name: codecov-umbrella - publish-test-report: needs: - backend-unit-test - backend-wmg-unit-test - - wmg-processing-unit-test - - processing-unit-test - e2e-test runs-on: ubuntu-20.04 steps: diff --git a/Makefile b/Makefile index 05bab6dede63e..28435699d762b 100644 --- a/Makefile +++ b/Makefile @@ -8,9 +8,7 @@ ifeq ($(AWS_ACCESS_KEY_ID),) export TEST_AWS_PROFILE ?= single-cell-dev endif export DEPLOYMENT_STAGE ?= test -COVERAGE_DATA_FILE=.coverage.$(shell git rev-parse --short HEAD) -COVERAGE_FILTER=--omit=*/dist-packages/*,*/backend/database/*,*/backend/scripts/*,*/site-packages/* --include=*/backend/*,*/tests/unit/* -export COVERAGE_RUN_ARGS:=--data-file=$(COVERAGE_DATA_FILE) --parallel-mode $(COVERAGE_FILTER) + .PHONY: fmt fmt: @@ -28,8 +26,8 @@ unit-test: local-unit-test .PHONY: wmg-processing-unittest wmg-processing-unittest: # This target is intended to be run INSIDE the wmg processing container - DEPLOYMENT_STAGE=test PYTHONWARNINGS=ignore:ResourceWarning coverage run $(COVERAGE_RUN_ARGS) -m pytest \ - tests/unit/wmg_processing/ --rootdir=. --alluredir=./allure-results --verbose; + DEPLOYMENT_STAGE=test PYTHONWARNINGS=ignore:ResourceWarning pytest tests/unit/wmg_processing \ + --rootdir=. --alluredir=./allure-results --verbose; .PHONY: functional-test functional-test: local-functional-test @@ -145,18 +143,15 @@ local-shell: ## Open a command shell in one of the dev containers. ex: make loca docker-compose exec $(CONTAINER) bash .PHONY: local-unit-test -local-unit-test: local-unit-test-backend local-unit-test-wmg-backend local-unit-test-wmg-processing local-unit-test-processing -# Run all backend and processing unit tests in the dev environment, with code coverage +local-unit-test: local-unit-test-backend local-unit-test-wmg-processing# Run all backend and processing unit tests in the dev environment, with code coverage .PHONY: local-unit-test-backend local-unit-test-backend: - docker-compose run --rm -T backend bash -c \ - "cd /single-cell-data-portal && coverage run $(COVERAGE_RUN_ARGS) -m pytest --alluredir=./allure-results tests/unit/backend/layers/"; + docker-compose run --rm -T backend bash -c "cd /single-cell-data-portal && python3 -m pytest tests/unit/backend/layers/"; .PHONY: local-unit-test-wmg-backend local-unit-test-wmg-backend: - docker-compose run --rm -T backend bash -c \ - "cd /single-cell-data-portal && coverage run $(COVERAGE_RUN_ARGS) -m pytest --alluredir=./allure-results tests/unit/backend/wmg/"; + docker-compose run --rm -T backend bash -c "cd /single-cell-data-portal && python3 -m pytest tests/unit/backend/wmg/"; .PHONY: local-integration-test-backend local-integration-test-backend: @@ -165,7 +160,7 @@ local-integration-test-backend: .PHONY: local-unit-test-processing local-unit-test-processing: # Run processing-unittest target in `processing` Docker container docker-compose $(COMPOSE_OPTS) run --rm -e DEV_MODE_COOKIES= -T processing \ - bash -c "cd /single-cell-data-portal && coverage run $(COVERAGE_RUN_ARGS) -m pytest --alluredir=./allure-results tests/unit/processing/"; + bash -c "cd /single-cell-data-portal && python3 -m pytest tests/unit/processing/"; .PHONY: local-unit-test-wmg-processing local-unit-test-wmg-processing: # Run processing-unittest target in `wmg_processing` Docker container @@ -223,15 +218,3 @@ local-uploadsuccess: .env.ecr ## Run the upload success lambda with a dataset id .PHONY: local-cxguser-cookie local-cxguser-cookie: ## Get cxguser-cookie docker-compose $(COMPOSE_OPTS) run --rm backend bash -c "cd /single-cell-data-portal && python login.py" - -.PHONY: coverage/combine -coverage/combine: - - docker-compose $(COMPOSE_OPTS) run --rm -T backend bash -c "cd /single-cell-data-portal && coverage combine --data-file=$(COVERAGE_DATA_FILE)" - -.PHONY: coverage/report -coverage/report-xml: coverage/combine - docker-compose $(COMPOSE_OPTS) run --rm -T backend bash -c "cd /single-cell-data-portal && coverage xml --data-file=$(COVERAGE_DATA_FILE) -i --skip-empty" - -.PHONY: coverage/report -coverage/report-html: coverage/combine - docker-compose $(COMPOSE_OPTS) run --rm -T backend bash -c "cd /single-cell-data-portal && coverage html --data-file=$(COVERAGE_DATA_FILE) -i --skip-empty" diff --git a/requirements.txt b/requirements.txt index f1ff1e28bf4b1..c25e0389092c5 100644 --- a/requirements.txt +++ b/requirements.txt @@ -5,7 +5,7 @@ black==22.3.0 # Must be kept in sync with black version in .pre-commit-config.y boto3>=1.11.17 botocore>=1.14.17 click==8.1.3 -coverage>=6.5.0 +coverage>=2.0.15 flake8-black==0.3.2 # Must be kept in sync with flake8 version in .pre-commit-config.yaml flake8==4.0.1 # Must be kept in sync with flake8 version in .pre-commit-config.yaml furl