-
Notifications
You must be signed in to change notification settings - Fork 1.1k
use pytest-mock to simplify and improve tests, improve test docs #468
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
Merged
Merged
Changes from all commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
783ddf3
replace PVSystem iam tests with mocked versions
wholmgren df7289d
add autospec
wholmgren 716bb71
add pytest-mock to ci
wholmgren 422e0b3
use mocker.spy for most tests
wholmgren 4e2fafc
mock PVSystem.pvwatts tests
wholmgren f947617
PVSystem.sapm mocked
wholmgren 546f134
remove unnecessary code in test_pvsystem
wholmgren 1354188
update contibuting testing recommendations
wholmgren 2a51f74
clean up
wholmgren 9c57d0b
more pvsystem mocks
wholmgren ac37a3a
add mock dc test to test_modelchain.py
wholmgren cccb42a
add contributing example for modelchain
wholmgren f8a90b0
update whatsnew
wholmgren 0e1a267
improve compatibility with older python
wholmgren 4d0668e
improve example test
wholmgren b80a1e4
merge upstream changes
wholmgren d0aecb3
add mocked PVSystem fs spectral loss test
wholmgren 5c80fd3
fix flake8 warnings
wholmgren c5b8f67
update for calcparams_desoto signature change
wholmgren 4bf3579
update PVSystem sapm eff irr params
wholmgren 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 hidden or 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 hidden or 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 |
---|---|---|
|
@@ -4,6 +4,7 @@ dependencies: | |
- pytz | ||
- pytest | ||
- pytest-cov | ||
- pytest-mock | ||
- nose | ||
- pip: | ||
- coveralls |
This file contains hidden or 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 |
---|---|---|
|
@@ -14,6 +14,7 @@ dependencies: | |
- siphon | ||
- pytest | ||
- pytest-cov | ||
- pytest-mock | ||
- nose | ||
- pip: | ||
- coveralls | ||
|
This file contains hidden or 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 |
---|---|---|
|
@@ -14,6 +14,7 @@ dependencies: | |
- siphon | ||
- pytest | ||
- pytest-cov | ||
- pytest-mock | ||
- nose | ||
- pip: | ||
- coveralls | ||
|
This file contains hidden or 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 |
---|---|---|
|
@@ -14,6 +14,7 @@ dependencies: | |
- siphon | ||
- pytest | ||
- pytest-cov | ||
- pytest-mock | ||
- nose | ||
- pip: | ||
- coveralls | ||
|
This file contains hidden or 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 |
---|---|---|
|
@@ -14,6 +14,7 @@ dependencies: | |
#- siphon | ||
- pytest | ||
- pytest-cov | ||
- pytest-mock | ||
- nose | ||
- pip: | ||
- coveralls |
This file contains hidden or 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 |
---|---|---|
|
@@ -9,7 +9,7 @@ contribute. | |
|
||
|
||
Easy ways to contribute | ||
----------------------- | ||
~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
||
Here are a few ideas for you can contribute, even if you are new to | ||
pvlib-python, git, or Python: | ||
|
@@ -33,7 +33,7 @@ pvlib-python, git, or Python: | |
|
||
|
||
How to contribute new code | ||
-------------------------- | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
||
Contributors to pvlib-python use GitHub's pull requests to add/modify | ||
its source code. The GitHub pull request process can be intimidating for | ||
|
@@ -81,29 +81,142 @@ changes, such as fixing documentation typos. | |
|
||
|
||
Testing | ||
------- | ||
~~~~~~~ | ||
|
||
pvlib's unit tests can easily be run by executing ``py.test`` on the | ||
pvlib directory: | ||
|
||
``py.test pvlib`` | ||
``pytest pvlib`` | ||
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. Older versions of pytest were called with py.test. Now it's just pytest. |
||
|
||
or, for a single module: | ||
|
||
``py.test pvlib/test/test_clearsky.py`` | ||
``pytest pvlib/test/test_clearsky.py`` | ||
|
||
While copy/paste coding should generally be avoided, it's a great way | ||
to learn how to write unit tests! | ||
or, for a single test: | ||
|
||
Unit test code should be placed in the corresponding test module in the | ||
pvlib/test directory. | ||
``pytest pvlib/test/test_clearsky.py::test_ineichen_nans`` | ||
|
||
Use the ``--pdb`` flag to debug failures and avoid using ``print``. | ||
|
||
New unit test code should be placed in the corresponding test module in | ||
the pvlib/test directory. | ||
|
||
Developers **must** include comprehensive tests for any additions or | ||
modifications to pvlib. | ||
|
||
pvlib-python contains 3 "layers" of code: functions, PVSystem/Location, | ||
and ModelChain. Contributors will need to add tests that correspond to | ||
the layer that they modify. | ||
|
||
Functions | ||
--------- | ||
Tests of core pvlib functions should ensure that the function returns | ||
the desired output for a variety of function inputs. The tests should be | ||
independent of other pvlib functions (see :issue:`394`). The tests | ||
should ensure that all reasonable combinations of input types (floats, | ||
nans, arrays, series, scalars, etc) work as expected. Remember that your | ||
use case is likely not the only way that this function will be used, and | ||
your input data may not be generic enough to fully test the function. | ||
Write tests that cover the full range of validity of the algorithm. | ||
It is also important to write tests that assert the return value of the | ||
function or that the function throws an exception when input data is | ||
beyond the range of algorithm validity. | ||
|
||
PVSystem/Location | ||
----------------- | ||
The PVSystem and Location classes provide convenience wrappers around | ||
the core pvlib functions. The tests in test_pvsystem.py and | ||
test_location.py should ensure that the method calls correctly wrap the | ||
function calls. Many PVSystem/Location methods pass one or more of their | ||
object's attributes (e.g. PVSystem.module_parameters, Location.latitude) | ||
to a function. Tests should ensure that attributes are passed correctly. | ||
These tests should also ensure that the method returns some reasonable | ||
data, though the precise values of the data should be covered by | ||
function-specific tests discussed above. | ||
|
||
We prefer to use the ``pytest-mock`` framework to write these tests. The | ||
test below shows an example of testing the ``PVSystem.ashraeiam`` | ||
method. ``mocker`` is a ``pytest-mock`` object. ``mocker.spy`` adds | ||
features to the ``pvsystem.ashraeiam`` *function* that keep track of how | ||
it was called. Then a ``PVSystem`` object is created and the | ||
``PVSystem.ashraeiam`` *method* is called in the usual way. The | ||
``PVSystem.ashraeiam`` method is supposed to call the | ||
``pvsystem.ashraeiam`` function with the angles supplied to the method | ||
call and the value of ``b`` that we defined in ``module_parameters``. | ||
The ``pvsystem.ashraeiam.assert_called_once_with`` tests that this does, | ||
in fact, happen. Finally, we check that the output of the method call is | ||
reasonable. | ||
|
||
.. code-block:: python | ||
def test_PVSystem_ashraeiam(mocker): | ||
# mocker is a pytest-mock object. | ||
# mocker.spy adds code to a function to keep track of how it is called | ||
mocker.spy(pvsystem, 'ashraeiam') | ||
|
||
# set up inputs | ||
module_parameters = {'b': 0.05} | ||
system = pvsystem.PVSystem(module_parameters=module_parameters) | ||
thetas = 1 | ||
|
||
# call the method | ||
iam = system.ashraeiam(thetas) | ||
|
||
# did the method call the function as we expected? | ||
# mocker.spy added assert_called_once_with to the function | ||
pvsystem.ashraeiam.assert_called_once_with(thetas, b=module_parameters['b']) | ||
|
||
# check that the output is reasonable, but no need to duplicate | ||
# the rigorous tests of the function | ||
assert iam < 1. | ||
|
||
Avoid writing PVSystem/Location tests that depend sensitively on the | ||
return value of a statement as a substitute for using mock. These tests | ||
are sensitive to changes in the functions, which is *not* what we want | ||
to test here, and are difficult to maintain. | ||
|
||
ModelChain | ||
---------- | ||
The tests in test_modelchain.py should ensure that | ||
``ModelChain.__init__`` correctly configures the ModelChain object to | ||
eventually run the selected models. A test should ensure that the | ||
appropriate method is actually called in the course of | ||
``ModelChain.run_model``. A test should ensure that the model selection | ||
does have a reasonable effect on the subsequent calculations, though the | ||
precise values of the data should be covered by the function tests | ||
discussed above. ``pytest-mock`` can also be used for testing ``ModelChain``. | ||
|
||
The example below shows how mock can be used to assert that the correct | ||
PVSystem method is called through ``ModelChain.run_model``. | ||
|
||
.. code-block:: python | ||
def test_modelchain_dc_model(mocker): | ||
# set up location and system for model chain | ||
location = location.Location(32, -111) | ||
system = pvsystem.PVSystem(module_parameters=some_sandia_mod_params, | ||
inverter_parameters=some_cecinverter_params) | ||
|
||
# mocker.spy adds code to the system.sapm method to keep track of how | ||
# it is called. use returned mock object m to make assertion later, | ||
# but see example above for alternative | ||
m = mocker.spy(system, 'sapm') | ||
|
||
# make and run the model chain | ||
mc = ModelChain(system, location, | ||
aoi_model='no_loss', spectral_model='no_loss') | ||
times = pd.date_range('20160101 1200-0700', periods=2, freq='6H') | ||
mc.run_model(times) | ||
|
||
# assertion fails if PVSystem.sapm is not called once | ||
# if using returned m, prefer this over m.assert_called_once() | ||
# for compatibility with python < 3.6 | ||
assert m.call_count == 1 | ||
|
||
# ensure that dc attribute now exists and is correct type | ||
assert isinstance(mc.dc, (pd.Series, pd.DataFrame)) | ||
|
||
|
||
This documentation | ||
------------------ | ||
~~~~~~~~~~~~~~~~~~ | ||
|
||
If this documentation is unclear, help us improve it! Consider looking | ||
at the `pandas | ||
|
This file contains hidden or 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 hidden or 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
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.
I think these needed to change to make the new level of organization work below (Functions, PVsystem/Location, ModelChain).