Skip to content
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

Streamline tests #128

Merged
merged 15 commits into from
Mar 12, 2019
Merged

Streamline tests #128

merged 15 commits into from
Mar 12, 2019

Conversation

khaeru
Copy link
Member

@khaeru khaeru commented Mar 7, 2019

tl;dr: this PR makes better use of pytest features instead of reinventing them in multiple places. It also moves some testing code to a new ixmp.testing submodule so it can be reused by message_ix (e.g. this shouldn't need redefinition) or by users.

I started making some of these changes when writing tests for #126 and figured they could stand alone.

  • ixmp.testing: new submodule for reuse of testing utilities.
    • create_local_testdb(): moved from tests/testing_utils.py
    • dantzig_transport(): replaces code in tests/test_integration.py and
      tests/testdb_setup.py.
  • consolidate test data in tests/data.
  • ixmp.Platform.init: ensure dbprops is string, for Java compat.
  • ixmp.default_paths: use Path objects.
  • tests/conftest.py:
    • new fixtures: test_data_path, tutorial_path.
    • existing test_mp* fixtures: add docstrings, use tmp_path_factory
      fixture.
    • new hook: pytest_report_header (replaces test_admin.py).
  • other tests: use pytest & ixmp fixtures.

PR checklist:

  • Tests added
  • Documentation added
  • Description in RELEASE_NOTES.md added

- ixmp.testing: new submodule for reuse of testing utilities.
  - create_local_testdb(): moved from tests/testing_utils.py
  - dantzig_transport(): replaces code in tests/test_integration.py and
    tests/testdb_setup.py.
- consolidate test data in tests/data.
- ixmp.Platform.__init__: ensure dbprops is string, for Java compat.
- ixmp.default_paths: use Path objects.
- tests/conftest.py:
  - new fixtures: test_data_path, tutorial_path.
  - existing test_mp* fixtures: add docstrings, use tmp_path_factory
    fixture.
  - new hook: pytest_report_header (replaces test_admin.py).
- other tests: use pytest & ixmp fixtures.
@khaeru khaeru requested a review from gidden March 7, 2019 10:35
@khaeru khaeru self-assigned this Mar 7, 2019
@khaeru khaeru added this to the 0.2 milestone Mar 7, 2019
@khaeru
Copy link
Member Author

khaeru commented Mar 7, 2019

Okay, finally all the tests pass. This is ready to review & merge, from my POV.

Copy link
Member

@danielhuppmann danielhuppmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @khaeru! two minor suggestions...

and one question: in the current setup, the local default HSQLDB is touched every time a user re-installs and runs the tests on the tutorial notebooks. So the default database files become quite bloated over time. Would it be possible to connect the tutorial-notebook tests to a temporary db?

ixmp/default_paths.py Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
@khaeru
Copy link
Member Author

khaeru commented Mar 12, 2019

Would it be possible to connect the tutorial-notebook tests to a temporary db?

As mentioned in Slack, this is possible, but it requires new functionality. Related to #124. I would prefer to merge this PR and then work on the other issue.

@danielhuppmann danielhuppmann merged commit 51ff58d into iiasa:master Mar 12, 2019
@khaeru khaeru deleted the testing branch April 1, 2019 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants