kizniche / Mycodo

An environmental monitoring and regulation system
http://kylegabriel.com/projects/
GNU General Public License v3.0
2.93k stars 492 forks source link

Create more test factories for each db model #143

Open etiology opened 7 years ago

etiology commented 7 years ago

As the testing suite develops there will be a need to create data in the database prior to running tests. Currently this is being done with the use of factory-boy for Users.

UserFactory Example:

class UserFactory(factory.Factory):
    """ A factory for creating user models """
    class Meta(object):
        model = models.Users

    user_name = factory.Faker('name')
    user_email = factory.Faker('email')

I suggest we add more factories, one for each database model. At a future point we may want to upgrade all of the factories to use the factory.alchemy.SQLAlchemyModelFactory subclass.

We may also want to add some crud methods to our base class for the models so that we can easily work with them after we generate them in a factory.

kizniche commented 7 years ago

I separated the factories with commit d9c4f42 (and 57e32b4, oops), and attempted to create a test to add a new sensor

@mock.patch('mycodo.mycodo_flask.authentication.views.login_log')
def test_adding_sensor_logged_in_as_admin(_, testapp, user_db, mycodo_db):
    """ Verifies behavior of these endpoints for a logged in admin user """
    # Create admin user and log in
    admin_user = create_user(user_db, 'admin', 'name_admin', 'secret_pass')
    login_user(testapp, admin_user.user_name, 'secret_pass')
    session['user_group'] = 'admin'

    # Create WTForms form and populate data
    formAddSensor = flaskforms.AddSensor()
    formAddSensor.sensor.data = 'RPiCPULoad'
    formAddSensor.numberSensors.data = 1

    # Submit form data to create sensor in database
    flaskutils.sensor_add(formAddSensor, display_order=[])
    # Verify data was entered into database (doesn't work)
    sensor = flaskutils.db_retrieve_table(current_app.config['MYCODO_DB_PATH'], Sensor)
    for each_sensor in sensor:
        print("01 Testing return sensor name: {}".format(each_sensor.name))

    # Alternate method to enter data into database
    new_sensor = SensorFactory()
    new_sensor.name = 'name'
    mycodo_db.add(new_sensor)
    mycodo_db.commit()
    # Verify data was entered into database (does work)
    sensor = flaskutils.db_retrieve_table(current_app.config['MYCODO_DB_PATH'], Sensor)
    for each_sensor in sensor:
        print("02 Testing return sensor name: {}".format(each_sensor.name))

What I was attempting to do was create the sensor database entry with flaskutils.sensor_add(), but that didn't work (the first print statement doesn't get executed). I then used the model for creating a user, and that worked (second statement prints with the name). I didn't want to get too involved with the second method for adding data to the database when there may just be something I'm missing from using the already-built function (sensor_add()) to get the first method to work.

I think it would be a more beneficial test if we can include the execution of sensor_add(). What are your thoughts about this and would this require refactoring or am I just missing something crucial to getting the first method to work?

etiology commented 7 years ago

I'm still going through it but it looks like the accessing the form from behind the UI isn't working. You can access the form directly using the testapp fixture, getting the page, filling out the form and submitting it just like the login_user function does.

The error I see is that the form validation never completes. The resulting error is flashed to the page but since we are behind the UI and interacting with the forms directly we won't see it.

This seems to work if we add an id field to the sensor form (in this example I called it new_sensor_form):

def test_adding_sensor_logged_in_as_admin(_, testapp, user_db, mycodo_db):
    """ Verifies behavior of these endpoints for a logged in admin user """
    # Create admin user and log in
    admin_user = create_user(user_db, 'admin', 'name_admin', 'secret_pass')
    login_user(testapp, admin_user.user_name, 'secret_pass')

    # Go to the sensor page and create a single CPU sensor
    form = testapp.get('/sensor').maybe_follow().forms['new_sensor_form']
    form['numberSensors'] = 1
    form.select(name='sensor', value='RPi')
    response = form.submit().maybe_follow()
    response.showbrowser()

    assert "RPi Sensor with ID" in response
    assert "successfully added" in response
etiology commented 7 years ago

We may want to wrap that whole create new sensor routine into it's own function like we did with the login_user. Maybe give it some default arguments so that we can later change which sensors we are making and the quantity.

add_sensor(sensor_type='RPi', qty=1):
...
kizniche commented 7 years ago

Great! That helps a lot and will get me started on form/database tests.

etiology commented 7 years ago

Here is the ID field change I made to get this to work: <form class="form-inline" id="new_sensor_form" method="post" action="/sensor">

kizniche commented 7 years ago

Alright. I see the terminal browser open and show the success flash message. However, I'm not seeing the print statement anymore

@mock.patch('mycodo.mycodo_flask.authentication.views.login_log')
def test_adding_sensor_logged_in_as_admin(_, testapp, user_db, mycodo_db):
    """ Verifies behavior of these endpoints for a logged in admin user """
    # Create admin user and log in
    admin_user = create_user(user_db, 'admin', 'name_admin', 'secret_pass')
    login_user(testapp, admin_user.user_name, 'secret_pass')

    # Go to the sensor page and create a single CPU sensor
    form = testapp.get('/sensor').maybe_follow().forms['new_sensor_form']
    form['numberSensors'] = 1
    form.select(name='sensor', value='RPi')
    response = form.submit().maybe_follow()
    # response.showbrowser()

    assert "RPi Sensor with ID" in response
    assert "successfully added" in response

    # Verify data was entered into database (doesn't work)
    sensor = flaskutils.db_retrieve_table(current_app.config['MYCODO_DB_PATH'], Sensor)
    for each_sensor in sensor:
        print("01 Testing return sensor name: {}".format(each_sensor.name))
etiology commented 7 years ago

My guess is that flaskutils.db_retrieve_table is returning an empty list.

If you mark the sensor = flaskutils.db_retrieve_table(current_app.config['MYCODO_DB_PATH'], Sensor) line with a red debugger entry point you can step through the code as it runs.

Yep. Just confirmed that return_table = new_session.query(table).all() is empty.

etiology commented 7 years ago

Found it.

One of several errors actually. flaskutils hard codes the MYCODO_DB_PATH into the session_scope call. The test application is using a throwaway db and this function is trying to use the production database.

etiology commented 7 years ago

This needs to use the current_app.config['MYCODO_DB_PATH'] trick. There are tons of them in this module so they all need to change.

kizniche commented 7 years ago

That makes sense. I had a hunch it was a db issue because we haven't updated the path. I suppose we'll have to fix all those before we can have a full test.

kizniche commented 7 years ago

Thanks, that was really helpful. I'm diving into webtest docs but haven't been able to find if it's possible to pull a list of options from the 'sensor' drop down field. Since not all form lists/multiselect fields are generated from WTForms (some are generated from the route), to ensure the correct options are used, this will need to be pulled from the page after it's rendered.

kizniche commented 7 years ago

I suppose it's probably best to just call the function that originally generated the choices instead of pulling from the rendered page.

kizniche commented 7 years ago

At a future point we may want to upgrade all of the factories to use the factory.alchemy.SQLAlchemyModelFactory subclass.

Do you think we're at a good point to do this in the dev-5.0 branch? The endpoint tests are currently broken, but I'm sure it's an obvious and small mistake on my part that's causing it.

etiology commented 7 years ago

yeah lets fix the tests. I see some issues here that we need to resolve early and maybe getting the tests up and running will flush out more issues that we can smooth over.

First isssue involves creating a new db instance object to interact with. In models we see the line that instantiates the new db instance: db = SQLAlchemy()

But with flask-sqlalchemy we have to be careful about doing this because the instance is not connected to the Flask Context at this point. In fact this object has no idea what your flask settings are yet. A better way its to use your flask extensions module to instantiate the extensions and then reference them there. I you instantiate them again then you now have two independent systems operating in your app but one doesn't work because it is working outside of the application context.