ladybug-tools / honeybee

:honeybee: A python library to create, run and visualize radiance studies. Core library of Honeybee[+].
http://ladybug-tools.github.io/honeybee/docs
GNU General Public License v3.0
93 stars 25 forks source link

Restructure testing framework and integrate to Travis #46

Closed mostaphaRoudsari closed 6 years ago

mostaphaRoudsari commented 6 years ago

@saeranv, what does it take to make this happen? The sooner we finish the integration the better for everyone. What is your experience from implementing it to Dragonfly repository?

saeranv commented 6 years ago

@mostaphaRoudsari I think it should be pretty easy. I used the test structure you have for Honeybee as the basis for UWG's tests, so there are very little differences between the two.

Things we need to do:

1. Sign up for a Travis CI account https.// travis-ci.org/

2. Add a .travis.yml file to configure the tests. Then if someone pushes a commit to the repo it should automatically run all the tests in the testing folder. This travis.yml is the very simple one I'm using for UWG.

language: python
python:
  - "2.7"
# command to install dependencies
#install:
#   - pip install -r requirements.txt
# command to run tests
script: pytest

notifications:
  email: never

As you can see I've told it to test for python 2.7, and to use pytest. If there's an error in the test code, the build status in the Travis CI interface will fail which is reflected in the status badge I've placed on the UWG readme: https://github.com/chriswmackey/urbanWeatherGen.

3. Switch the testing library to pytest and modify the assertion statements. Travis CI can run tests with unittest, but if you want to switch to pytest, then the transition should be fairly simple. I think the assertion statements will have to change, because right now you are inheriting unittest assertion statements, but other then a syntax change, they are functionally the same as pytest assertion statements.

unittest:

self.assertEqual(self.epw2wea.output_wea_file, self.test_wea)

pytest:

assert self.forcIP.deepTemp == pytest.approx(299.8392473118278, abs=1e-12)

4. Change the naming convention of your existing tests so that pytest can discover them. These are the rules for test discovery in pytest. Basically pytest looks for files named test_*.py or *_test.py. Classes must start with Test, and can't have an __init__ statement. Here's an example of a test class I set up for UWG: https://github.com/chriswmackey/urbanWeatherGen/blob/master/tests/test_forcing.py.

So those are the four changes I think we need to make. I think the best way to begin would be to take one of your simpler tests: https://github.com/ladybug-tools/honeybee/blob/master/tests/radiance_command_epw2wea_test.py and set up the whole Travis CI framework for that. Do you want me to try that out or would you prefer to do it yourself?

-S

mostaphaRoudsari commented 6 years ago

Hi @saeranv, Thank you for the comprehensive reply. If you can get started I will follow your lead. On item 4 my understanding is that pytest should be able to handle unittest by default without making changes to the classes. See this: https://docs.pytest.org/en/3.0.0/unittest.html#autouse-fixtures-and-accessing-other-fixtures

AntoineDao commented 6 years ago

Quick question about testing framework: how do you propose to go about testing honeybee and ladybug together? Quite a few methods in HB[+] depend on ladybug methods. Would you have two repos in one folder and copy the tests out of the honyeebee folder into the parent folder? Do you copy ladybug into the honeybee folder and run tests from honeybee repo directory?

saeranv commented 6 years ago

@mostaphaRoudsari Good point, I'll check out that link. Just to clarify, does this mean you want to keep all the HB[+] unittests the same and not transition them over to pytest?

@AntoineDao If I'm understanding your question correctly, we can just treat ladybug as a python library that we import when we execute a HB+ script. As long as Ladybug is in our python path everything will work. Or do you mean what are the test "strategies" for testing HB and LB together?

mostaphaRoudsari commented 6 years ago

@saeranv, No. I think we should change the unittest to pytest but it should happen gradually while we're developing and adding/revising the test units. Didn't want to add one extra task right now when it can be forgiven at this point. MVP for the win! :)

@AntoineDao, I haven't used Travis but based on the yml sample up there it should work similar to docker-compose and creates the environment from scratch each time to run the testunits.

If we want to use pip install and requirements.txt we have to figure out the naming convention and then make it happen. I was not able to make a decision on my own. See this: https://github.com/ladybug-tools/honeybee-server/issues/6#issuecomment-353805900

cc: @chriswmackey

saeranv commented 6 years ago

Update: I tested pytest and travis-ci on my fork of honeybee and pytest is working well on my local drive with some minor modifications to the test class: image

However, I'm running into some problems with importing the ladybug module. Specifically, my local repo can't importing the ladybug module when running the tests. I managed to solve this problem on my local repo by modifying the import module in honeybee\__init__.py to lead to my copy of ladybug: sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), '..','..',lib))) - but I doubt this is the best way to fix this issue.

I run my python modules from the main honeybee directory with the following command: python -m tests.radiance_command_epw2wea_test or in the case of pytest: pytest tests/radiance_command_epw2wea_test.py ...which I thought is the correct way to call python modules while making sure the init file will import all the correct modules. Let me know if I'm doing something wrong here @mostapha, in the meantime I'll check stack overflow.

Also travis-ci is syncing correctly with the pytest, but unless we want to simply copy ladybug into the honeybee folder for now, we can't run the tests (as @AntoineDao indicated) until we can get it on pip and specify it on our requirements.txt.

mostaphaRoudsari commented 6 years ago

@saeranv, you can use pip install to install ladybug directly from the GitHub repository. We just need to add setup.py to repo. That should solve your problem with the dependencies.

https://github.com/pypa/pip/issues/3610

saeranv commented 6 years ago

@mostaphaRoudsari awesome! I didn't know that we can pip install directly from git. I just did a test with the method they listed in that issue and it looks like it's solving our problem on travis.

image ... image

I don't understand everything that is happening in the setup.py file, so I'd like to look at it again in more depth, and I haven't tested this on my local repo yet (just on travis' virtualenv) so there are a couple more things to test out before I submit a PR.

saeranv commented 6 years ago

@mostaphaRoudsari I have found a new problem :(

While with the requirements.txt and setup.py, travis is dowloading ladybug, it isn't going through and loading all the modules.

Below is the error message, which is showing that for the radiance_recipe_solaraccess_test.py file, it isn't able to find the analysis module in Ladybug.

$ python --version
Python 2.7.14
$ pip --version
pip 9.0.1 from /home/travis/virtualenv/python2.7.14/lib/python2.7/site-packages (python 2.7)
install
2.40s$ pip install -r requirements.txt
Collecting ladybug (from -r requirements.txt (line 1))
  Downloading ladybug-0.0.2.tar.gz
Building wheels for collected packages: ladybug
  Running setup.py bdist_wheel for ladybug ... done
  Stored in directory: /home/travis/.cache/pip/wheels/40/c7/8c/937d8faeb9fdb97b2a8ba001714dadc2dddd14dbcf6e3e3428
Successfully built ladybug
Installing collected packages: ladybug
Successfully installed ladybug-0.0.2
1.25s$ pytest
============================= test session starts ==============================
platform linux2 -- Python 2.7.14, pytest-3.3.0, py-1.5.2, pluggy-0.6.0
rootdir: /home/travis/build/saeranv/honeybee-1, inifile:
collected 4 items / 1 errors                                                   
==================================== ERRORS ====================================
__________ ERROR collecting tests/radiance_recipe_solaraccess_test.py __________
ImportError while importing test module '/home/travis/build/saeranv/honeybee-1/tests/radiance_recipe_solaraccess_test.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
tests/radiance_recipe_solaraccess_test.py:2: in <module>
    from honeybee.radiance.recipe.solaraccess.gridbased import SolarAccessGridBased
honeybee/radiance/recipe/solaraccess/gridbased.py:2: in <module>
    from .._gridbasedbase import GenericGridBased
honeybee/radiance/recipe/_gridbasedbase.py:8: in <module>
    from ..analysisgrid import AnalysisGrid
honeybee/radiance/analysisgrid.py:5: in <module>
    from ..schedule import Schedule
honeybee/schedule.py:7: in <module>
    from ladybug.analysisperiod import AnalysisPeriod
E   ImportError: No module named analysisperiod
!!!!!!!!!!!!!!!!!!! Interrupted: 1 errors during collection !!!!!!!!!!!!!!!!!!!!
=========================== 1 error in 0.56 seconds ============================

So the problem is, why isn't the setup file (or requirements.txt) loading all the modules in ladybug correctly, and how to correct it. I'll keep investigating.

-S

saeranv commented 6 years ago

@mostaphaRoudsari

Okay I've finally solved the ladybug import problem, and can now successfully load modules from the ladybug repo for honeybee tests. You can see in the image below, we can pip install ladybug from it's github, and it's successfully testing the radiance_view_test.py (where I have added some ladybug methods for testing). image

The main conflicts that were preventing this with my earlier setup is that:

  1. For some reason installing the ladybug via github through requirements.txt didn't work, but moving the same link into the .travis.yml worked (line 441 in the image above).
  2. There needs to be a setup.py in ladybug uses the find_packages() module to recursively install all it's nested directories:
    
    from setuptools import setup, find_packages

setup( name='ladybug', packages=find_packages() )


I can send a PR with the .travis.yml, and setup.py, and conftest.py that I'm using. That should be all we need at this point to get our Travis mvp started. One thing to note is that I believe most of the tests are very obsolete and will fail our Travis build until we refactor them or manually exclude them using the conftest.py file. 

S 
AntoineDao commented 6 years ago

Travis has been integrated as well as coveralls in #233. We still need to write tests to make the build pass but I reckon that's a seperate issue to this one which was more focused on Travis functionality etc... Feel free to re-open if anyone disagrees.