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

Add pyroapi.tests module with example usage #3

Merged
merged 3 commits into from
Oct 10, 2019
Merged

Conversation

fritzo
Copy link
Member

@fritzo fritzo commented Oct 10, 2019

Addresses #2

This adds an importable module pyroapi.tests that depends on pytest. I expect this to eventually be similar to test_valid_models.py. We can later split this into pyroapi.tests.cheap and pyroapi.tests.expensive if needed. I've added an example conftest.py and test_tests.py do demonstrate use of pyroapi.tests.

I am a little dissatisfied that pyroapi.tests depends on pytest, but I believe we can make this an optional dependency, only required for users who want to use that component.

I am very happy with the terse usage pattern. Results look nice for the one test I've added:

test/test_testing.py::test_optimizer[pyro-Adam-py] <- pyroapi/tests.py PASSED            [  6%]
test/test_testing.py::test_optimizer[pyro-Adam-jit] <- pyroapi/tests.py PASSED           [ 12%]
test/test_testing.py::test_optimizer[pyro-ClippedAdam-py] <- pyroapi/tests.py PASSED     [ 18%]
test/test_testing.py::test_optimizer[pyro-ClippedAdam-jit] <- pyroapi/tests.py PASSED    [ 25%]
test/test_testing.py::test_optimizer[minipyro-Adam-py] <- pyroapi/tests.py PASSED        [ 31%]
test/test_testing.py::test_optimizer[minipyro-Adam-jit] <- pyroapi/tests.py PASSED       [ 37%]
test/test_testing.py::test_optimizer[minipyro-ClippedAdam-py] <- pyroapi/tests.py XFAIL  [ 43%]
test/test_testing.py::test_optimizer[minipyro-ClippedAdam-jit] <- pyroapi/tests.py XFAIL [ 50%]
test/test_testing.py::test_optimizer[numpy-Adam-py] <- pyroapi/tests.py XFAIL            [ 56%]
test/test_testing.py::test_optimizer[numpy-Adam-jit] <- pyroapi/tests.py XFAIL           [ 62%]
test/test_testing.py::test_optimizer[numpy-ClippedAdam-py] <- pyroapi/tests.py XFAIL     [ 68%]
test/test_testing.py::test_optimizer[numpy-ClippedAdam-jit] <- pyroapi/tests.py XFAIL    [ 75%]
test/test_testing.py::test_optimizer[funsor-Adam-py] <- pyroapi/tests.py XFAIL           [ 81%]
test/test_testing.py::test_optimizer[funsor-Adam-jit] <- pyroapi/tests.py XFAIL          [ 87%]
test/test_testing.py::test_optimizer[funsor-ClippedAdam-py] <- pyroapi/tests.py XFAIL    [ 93%]
test/test_testing.py::test_optimizer[funsor-ClippedAdam-jit] <- pyroapi/tests.py XFAIL   [100%]

Tested

import pytest


def pytest_runtest_call(item):
Copy link
Member Author

Choose a reason for hiding this comment

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

this is more automatic than our old xfail_if_not_implemented()


@pytest.mark.parametrize("jit", [False, True], ids=["py", "jit"])
@pytest.mark.parametrize("optim_name", ["Adam", "ClippedAdam"])
def test_optimizer(backend, optim_name, jit):
Copy link
Member Author

Choose a reason for hiding this comment

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

Note this backend arg must be provided by a user-defined fixture, as in the tests.

Copy link
Member

@neerajprad neerajprad left a comment

Choose a reason for hiding this comment

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

Looks great to me!

I am a little dissatisfied that pyroapi.tests depends on pytest, but I believe we can make this an optional dependency, only required for users who want to use that component.

I agree. For as long as the main module doesn't depend on tests, having a dependency on pytest is okay. I expect that most users of these cases (at least in the near term) will be internal users who are using pytest as their test runner, so it doesn't seem like a big deal.

@neerajprad neerajprad merged commit 08923ef into master Oct 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants