feat: split the runner and tests #60
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## dev #60 +/- ##
==========================================
- Coverage 99.73% 99.61% -0.13%
==========================================
Files 12 10 -2
Lines 754 516 -238
==========================================
- Hits 752 514 -238
Misses 2 2
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
This is mostly fine. Still a few mentions of TES, GA4GH etc. though. Plus it should be clear that the runner is for OpenAPI specs, not just any old API spec.
The most critical issue here, in my view, is already addressed in the review for ga4gh/compliance-tests-ga4gh-tes#1: I really think the runner is the tool and should contain the entry point to run the tool (rather than putting an entry point in each test suite). This avoids code repetition and keeps the test suites to a minimum (ideally shouldn't contain any code, but guess models/contstants can't be easily avoided).
|
|
||
| The TES Compliance Suite determines a server's compliance with the [TES API specification][res-tes-spec]. The specification has been developed by the [Global Alliance for Genomics and Health][res-ga4gh], an international coalition, formed to enable the sharing of genomic and clinical data. It serves to provide a standardized API framework and data structure to allow for interoperability of datasets hosted at different institutions. | ||
| The API Compliance Suite determines a server's compliance with the [TES API specification][res-tes-spec]. The specification has been developed by the [Global Alliance for Genomics and Health][res-ga4gh], an international coalition, formed to enable the sharing of genomic and clinical data. It serves to provide a standardized API framework and data structure to allow for interoperability of datasets hosted at different institutions. |
There was a problem hiding this comment.
This still mentions TES and GA4GH. Also, this test suite is only for OpenAPI-based services, isn't it? So please remove any mention of TES and GA4GH across the repo and replace with OpenAPI. You can (and probably should) of course link to some example specs, like TES and WES for which the runner has been tested (perhaps you could add a list of API specs for which compatible tests are available and then we can update that list as more specs become available), but it needs to be clear that these are just examples, and that in principle any OpenAPI spec will do.
Btw, do you think the test runner would work for OpenAPI 2.x specs as well? Or only 3.x? The docs should also indicate that. If you don't know, say that it was written for 3.x, but it may work for 2.x (but hasn't been tested/verified).
| @@ -2,9 +2,9 @@ | |||
| [![Coverage][badge-coverage]][badge-url-coverage] | |||
| [![Python 3.6][badge-python]](https://www.python.org) | |||
|
|
|||
| # TES Compliance Suite | |||
| # API Compliance Suite | |||
| @@ -45,7 +42,7 @@ | |||
| "properties": { | |||
| "$ref":{ | |||
| "type": "string", | |||
| "description": "The relative path to a test template." | |||
| "description": "The relative path from root to a test template." | |||
There was a problem hiding this comment.
This is potentially ambiguous. What is the root here? On a Linux file system, typically the root is /, but that would mean you'd expect an absolute, not relative path here. Also, it would actually be great if relative paths were supported, in addition to relative paths (which should, based on the principle of least surprise, rather be anchored to the user's current working directory). But this can also be done in a future issue/PR.
docs/test_config/common_schema.json
Outdated
There was a problem hiding this comment.
See comment here about location of schemas: ga4gh/compliance-tests-ga4gh-tes#1
|
Which one to look at, this one or #62? How do they differ? Best to keep everything in one PR, because this one is already reviewed quite a bit. Starting again on #62 would be painful, as I don't see what has changed (no need to look at the already reviewed stuff). If you wanna keep a branch without the latest changes, just clone the branch and push (without a PR), then apply the latest changes here |
Fixes #2
The compliance suite would be imported as a module and used within a test repo. The entry point would be inside the test repo where the tests will be run. The CLI commands will not work in the compliance suite from now onwards.