magfest-archive / hotel

MAGFest hotel room management
GNU Affero General Public License v3.0
1 stars 2 forks source link

Unit tests are failing on setup and teardown approvals #74

Closed RobRuana closed 7 years ago

RobRuana commented 7 years ago

See this travis build for details (also copied below). Yay automated tests!

You can quickly reproduce this from the command line with:

$ tox -e py34 -- -k TestHotelRequests

This is a failure of business logic that I don't understand.

py34 runtests: commands[6] | coverage run --source hotel -m py.test
============================= test session starts ==============================
platform linux -- Python 3.4.4, pytest-3.0.7, py-1.4.33, pluggy-0.4.0
rootdir: /home/travis/build/magfest/hotel, inifile:
collected 19 items 
hotel/tests/test_attendee_properties.py ...FF
hotel/tests/test_templates.py ..............
=================================== FAILURES ===================================
______________ TestHotelRequests.test_decline_setup_and_teardown _______________
self = <hotel.tests.test_attendee_properties.TestHotelRequests object at 0x7f265b0f46a0>
hotel_request_and_approve_setup_teardown = <  Hotel Requests>
    def test_decline_setup_and_teardown(self, hotel_request_and_approve_setup_teardown):
        """
            if someone has previously been approved for their setup/teardown hotel request,
            then later declines it, we still allow them to sign up for setup/teardown shifts
            "If someone ends up being approved for setup nights and then declines their hotel space, it might just mean
            that they decided to room with a friend or something. Allowing them to still take setup shifts seems
            reasonable in that case." see https://github.com/magfest/hotel/issues/18
            """
        assert hotel_request_and_approve_setup_teardown.attendee.can_work_setup
>       assert hotel_request_and_approve_setup_teardown.attendee.can_work_teardown
E       AssertionError: assert False
E        +  where False = <Attendee full_name=' '>.can_work_teardown
E        +    where <Attendee full_name=' '> = <  Hotel Requests>.attendee
hotel/tests/test_attendee_properties.py:61: AssertionError
____ TestHotelRequests.test_hotel_approved_and_can_work_setup_and_teardown _____
self = <hotel.tests.test_attendee_properties.TestHotelRequests object at 0x7f265b0c0a90>
hotel_request_and_approve_setup_teardown = <  Hotel Requests>
    def test_hotel_approved_and_can_work_setup_and_teardown(self, hotel_request_and_approve_setup_teardown):
        assert hotel_request_and_approve_setup_teardown.attendee.can_work_setup
>       assert hotel_request_and_approve_setup_teardown.attendee.can_work_teardown
E       AssertionError: assert False
E        +  where False = <Attendee full_name=' '>.can_work_teardown
E        +    where <Attendee full_name=' '> = <  Hotel Requests>.attendee
hotel/tests/test_attendee_properties.py:69: AssertionError
===================== 2 failed, 17 passed in 0.78 seconds ======================
ERROR: InvocationError: '/home/travis/build/magfest/hotel/.tox/py34/bin/coverage run --source hotel -m py.test'
___________________________________ summary ____________________________________
ERROR:   py34: commands failed
The command "tox -e $TOX_ENV" exited with 1.
Done. Your build exited with 1.
EliAndrewC commented 7 years ago

Ok, so here's what's going on with the business logic.

MAGFest offers hotel space to returning volunteers. By default, this hotel space is just for the nights of the actual event, e.g. for MAGFest itself that means Thur / Fri / Sat nights.

We also need people to come in before the event to help set up and stay afterwards to help tear down. Therefore, when we ask people what nights they want, we allow them to request setup nights and teardown nights, e.g. Wed / Sun. Since we have WAY more people who want to work setup/teardown than we need, those nights need to be approved by an admin.

The way this is reflected in the database is that we create a HotelRequests row in the database which foreign keys into the Attendee table with the nights they've requested, and by default it's approved column is false.

The setup and teardown shifts in the database are also each special kinds of shifts which we don't display by default, because volunteers need to be approved to take them. This is represented by the can_work_setup and can_work_teardown boolean database columns we add to the Attendee table.

What these failing tests are checking is the desired behavior that when we approve someone's hotel requests, it's supposed to automatically approve them for setup / teardown shifts based on what nights they requested.

At a glance, the reason why this isn't working is that the monkeypatching/mocking wasn't done correctly. In particular, what's happening seems to be that we're basically saying

hr = HotelRequests()
hr.attendee = Attendee()

and then calling some methods on hr.attendee but the attendee.hotel_requests backref isn't working because these objects were never added to the database and that relationship wasn't fully set up.

RobRuana commented 7 years ago

Thank you for the fantastic explanation!