-
Notifications
You must be signed in to change notification settings - Fork 30
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
Server tests #502
Merged
Merged
Server tests #502
Changes from 18 commits
Commits
Show all changes
36 commits
Select commit
Hold shift + click to select a range
849b52f
write all tests
hasan7n 152862a
change: remove unused endpoints
hasan7n a9abb6d
change: fix PUT user permission
hasan7n 21c8cef
change: fix PUT result behaviour
hasan7n 85887ba
change: remove URLs from cube uniqueness criteria
hasan7n fae91c0
change: apply mlcube rules on both PUT and POST
hasan7n 76b05ab
improve readability
hasan7n 14ecc83
change: modify how associations list works
hasan7n 0195b36
change: fix default approval status
hasan7n f4ff5df
change: fix approved_at update
hasan7n 7695dab
add notes and TODOs
hasan7n 0710c44
update workflow file
hasan7n c375225
modify client behaviour for model association list
hasan7n b3d7f7f
add some documentation for writing tests
hasan7n 2210f69
update workflow file
hasan7n d87a3b1
update server readme
hasan7n f89e706
Merge remote-tracking branch 'upstream/main' into server-tests
hasan7n 43430f5
empty
hasan7n a6e3b51
change: fix mlcube associations list permissions
hasan7n f043efa
change: modify result PUT and DELETE
hasan7n 239771c
change: fix permissions of approving the benchmark
hasan7n d06ad11
change: prevent updating a benchmark to PENDING
hasan7n 58e505e
Revert "change: remove unused endpoints"
hasan7n d480dc8
change: restrict marking benchmark as operation
hasan7n da4042d
change: demo dataset url is required
hasan7n e460be7
change: fix delete permissions for associations
hasan7n 74cedbf
change: server check for dataset assoc prep mlcube
hasan7n 63f6361
change: client side new association logic
hasan7n 6aa5ebd
change: make client side demo url required
hasan7n a528299
cleanup TODOs and related minor changes
hasan7n 6846dd4
fix linter issues
hasan7n eb36d9e
use a better datetime parser
hasan7n c7b6578
change integration tests order of model priority
hasan7n a5ecee2
stop using benchmark.models for permission reasons
hasan7n 1687b34
add Result's new fields to the client
hasan7n 8957464
refactor to fix linter issue
hasan7n File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
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
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
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
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
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,3 +1,64 @@ | ||
# Server | ||
|
||
Documentation TBD | ||
|
||
## Writing Tests | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I love that you added this! Thanks for the documentation Hasan! |
||
|
||
Each endpoint must have a test file. An exception is for the endpoints defined in the utils folder, one single file contains tests for all its endpoints. | ||
|
||
### Naming conventions | ||
|
||
A test file in a module is named according to the relative endpoint it tests. For example, the test files for the `/datasets/` and `/benchmarks/` endpoints (POST and GET list) are named as `test_.py`. The test file for `/results/<pk>/` endpoint is named as `test_pk.py`. | ||
|
||
### What to keep in mind when testing | ||
|
||
Testing an endpoint means testing, for each HTTP method it supports, the following: | ||
|
||
- Serializer validation rules (`serializers.py`) | ||
- Database constraints (`models.py`) | ||
- Permissions (referred to in `views.py`) | ||
|
||
Testing is focused on the actions that are not expected to happen, and focuses less on the actions that can happen (as an example, the tests should ensure that an unauthenticated user cannot access an endpoint, but they may not ensure that a certain type of user can edit a certain field.) | ||
|
||
### How tests should work | ||
|
||
Each test class should inherit from `MedPerfTest`, which sets up the local authentication and provides utils to create assets (users, datasets, mlcubes, ...) | ||
|
||
Each test class contains at least one test function. Both test classes and test class functions can be parameterized. **Each instance of a parameterized test is run independantly**; a new fresh database is used and the class's `SetUp` method is called prior to each test execution. | ||
|
||
### Running tests | ||
|
||
#### Run the whole tests | ||
|
||
To run the whole tests, run: | ||
|
||
```bash | ||
python manage.py test | ||
``` | ||
|
||
use the `--parallel` option to parallelize the tests. | ||
|
||
```bash | ||
python manage.py test --parallel | ||
``` | ||
|
||
#### Run individual files | ||
|
||
You can run individual tests files. For example: | ||
|
||
```bash | ||
python manage.py test dataset.tests.test_pk | ||
``` | ||
|
||
#### Run individual tests | ||
|
||
Running individual test classes or test functions can be done as follows. Example: | ||
|
||
```bash | ||
python manage.py test benchmark.tests.test_ -k BenchmarkPostTest | ||
python manage.py test benchmark.tests.test_ -k test_creation_of_duplicate_name_gets_rejected | ||
``` | ||
|
||
### Debugging tests | ||
|
||
Tests are not "unittests". For example, the test suite for `dataset` relies on the `mlcube` functionalities. This is because the `dataset` tests use utils to create a preparation MLCube for the datasets. When debugging, it might be useful to run test suites in a certain order, and use the `--failfast` option to exit on the first failure. A script is provided for this: `debug_tests.sh`. |
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
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
File renamed without changes.
Oops, something went wrong.
Oops, something went wrong.
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.
Should we handle filtering within the comms layer? Or within the entity layer? I think we've done filtering on the entity layer so far, because we also need to apply filtering to local entities
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's tricky for associations since we don't have an association
Entity
on the client yet.Filtering associations requires two steps:
Entity
, but for now I think this should live in the comms layer, since the client doesn't actually care about the history of associations. Also, this logic is required for all places in the client code where this comms function is called.a.
entity.list
command: which already applies filtering in the commands layer.b.
benchmark.get_models_uids
: which by definition should return approved associations model IDs. We can apply filtering inside this function.I think this will become cleaner if we decide to create an association
Entity
in our code base.