2 add unittests #5

josvandervelde · 2023-01-13T09:31:18Z

My first OpenML PR!

Unittests added
Changed the main file to make mocking possible
Some formatting changes (such as README, not much has actually changed)
Convenience scripts added in a scripts dir (I always like this. Downside: should be updated alongside the README)
While testing I also tested the error handling, and made some functional changes there, so that a response is given with a 400/500-range response code. Previously, some errors were ignored (e.g. duplicate entries) or a 200-response with an error in the body was returned.
No other functional changes (except error handling)
2 TODOs added, preferable removed before merging (to discuss)

…hat this is unnecessary

…erplate.

…nges

…d unittest for post_register; added sql constraints for post_register

PGijsbers

re: formatting

Because this is now a 2+ people repo (and because now there is time), it probably makes sense to set up some shared formatting configuration for the project. This avoids excessive diffs in the future. I'd suggest black and add a pre-commit hook, but I'm open to other alternatives :). See e.g., openml-python's pre-commit (though I think it would be better if we move the tool configuration itself to a pyproject.toml file).

scripts/run_apiserver.sh

scripts/run_mysql_server.sh

src/connectors/openml.py

src/tests/test_get_dataset_openml.py

src/tests/test_get_publication.py

src/tests/test_get_publications.py

src/tests/test_post_register_dataset.py

…tion. Ran auto-format on all files

src/connectors/openml.py

PGijsbers

🎉

josvandervelde added 11 commits January 12, 2023 11:23

Removed initialization of database in database/__init__.py. I think t…

1fc1b74

…hat this is unnecessary

Added a scripts directory for convenience

dd5b16b

Created the first unittest, for 'get_dataset'. Added some TODOs.

a175a98

Testcase for get_datasets

8eef237

Using pytests instead of unittests (after tip from Pieter). Less boil…

bcaec30

…erplate.

Mount the src directory in the docker, so that it gets updated on cha…

db2753b

…nges

Bugfixes so that the server works again (not only in unittests); adde…

03666aa

…d unittest for post_register; added sql constraints for post_register

Unittests for get_publications

519ef52

Unittests for get_publication

b2dfbdf

Test using weird characters as dataset name

f20f8b6

Cleanup. Added docstrings, type hints and 2 extra testcases.

29bd2fd

josvandervelde linked an issue Jan 13, 2023 that may be closed by this pull request

Add Unittests #2

Closed

josvandervelde requested a review from PGijsbers January 13, 2023 09:31

PGijsbers suggested changes Jan 13, 2023

View reviewed changes

josvandervelde added 9 commits January 13, 2023 14:12

Pre-commit configuration with auto-format, based on OpenMLs configura…

7d392bf

…tion. Ran auto-format on all files

Removed paths from the scripts

905253f

Session does not interact with the mocked_requests

4319180

Parametrized some tests

345277f

Typo: 'ds' -> 'pub'

abfbafd

Run pytest in pre-commit hook

7ec85da

Return more information when registering duplicated Dataset

0b88e53

Test all possible missing values

dfb6c1b

Removed TODOs from the code

625abfc

PGijsbers reviewed Jan 16, 2023

View reviewed changes

src/connectors/openml.py Show resolved Hide resolved

josvandervelde added 4 commits January 16, 2023 14:43

Making sure scripts work on macOS

5a0c5f3

Removed unnecessary nesting in testfile

05e5b20

Created pyproject.toml; moved black's configuration into pyproject.toml

32f0790

Added non-root user to dockerfile

443725b

PGijsbers approved these changes Jan 17, 2023

View reviewed changes

josvandervelde merged commit 0fe662d into main Jan 17, 2023

josvandervelde deleted the 2-add-unittests branch January 17, 2023 16:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

2 add unittests #5

2 add unittests #5

josvandervelde commented Jan 13, 2023

PGijsbers left a comment

PGijsbers left a comment

2 add unittests #5

2 add unittests #5

Conversation

josvandervelde commented Jan 13, 2023

PGijsbers left a comment

Choose a reason for hiding this comment

PGijsbers left a comment

Choose a reason for hiding this comment