Skip to content

Commit

Permalink
Merge pull request #49 from uc-cdis/improve_docs
Browse files Browse the repository at this point in the history
Improve docs / comment on missing validation
  • Loading branch information
pieterlukasse committed Jul 18, 2022
2 parents dfbcac9 + a593d9d commit 69e7613
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 6 deletions.
7 changes: 1 addition & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,6 @@ curl -d '{"variables":[{"variable_type": "concept", "concept_id": 2000000324},{"
```
- PRs to `master` get the docker image built on quay (via github action). See https://quay.io/repository/cdis/cohort-middleware?tab=tags
- The following config file determines which branch or tag is used on QA: https://github.com/uc-cdis/gitops-qa/blob/master/qa-mickey.planx-pla.net/manifest.json
- Once the image is built, it can be pulled. E.g. for branch `branch1`: `docker pull quay.io/cdis/cohort-middleware:branch1`
- If testing on QA:
- ssh to QA machine
- run the steps below:
Expand All @@ -170,13 +169,9 @@ curl -d '{"variables":[{"variable_type": "concept", "concept_id": 2000000324},{"

## Test the endpoints on QA

Examples:
Example:
```
curl -H "Content-Type: application/json" -H "$(cat auth.txt)" https://<qa-url-here>/sources | python -m json.tool
curl -H "Content-Type: application/json" -H "$(cat auth.txt)" https://<qa-url-here>/cohortdefinition-stats/by-source-id/2 | python -m json.tool
curl -d '{"ConceptIds":[2000000324,2000006885]}' -H "Content-Type: application/json" -H "$(cat auth.txt)" -X POST https://<qa-url-here>/cohort-data/by-source-id/2/by-cohort-definition-id/3
```

**Note that** the `<qa-url-here>` in these examples above needs to be replaced, and the ids used (`by-source-id/2`, `by-cohort-definition-id/3`) need
Expand Down
8 changes: 8 additions & 0 deletions tests/models_tests/models_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,14 @@ func TestRetrieveBreakdownStatsBySourceIdAndCohortIdWithResults(t *testing.T) {
}
}

// Tests what happens when persons have more than 1 HARE. This is a "data error" and should not
// happen in practice. The ideal solution would be for cohort-middleware to throw an error
// when it detects such a situation in the RetrieveBreakdownStats methods. This test shows that
// the code does not "hide" the error but instead returns the extra hare as an
// extra count, making the cohort numbers inconsistent and hopefully making the "data error" easy
// to spot.
// TODO - adjust the code to detect the issue and return an error, ideally with minimized or no repetition
// of the heavy queries in the RetrieveBreakdownStats methods...
func TestRetrieveBreakdownStatsBySourceIdAndCohortIdWithResultsWithOnePersonTwoHare(t *testing.T) {
setUp(t)
breakdownConceptId := hareConceptId
Expand Down

0 comments on commit 69e7613

Please sign in to comment.