Use datetime.utcnow in test data

[martinpeck]

I believe this pull request will remove some of the issues currently seen in running unit tests outside of docker. A class of unit test check that submitting Assignments after the due date, and after the lock date, generate an HTTP 403 response in the API. At times, these tests pass when the shouldn't. This change should fix this issue.

Note: This PR addresses one small issue...that some unit tests fail when run in a timezone that isn't UTC, and particularly if run outside of docker on a developer's local machine. I believe there's a bigger issue with regard to uniformly handling dates, times and timezones. This PR doesn't address this bigger issue.

The following tests were failing (for me) because they were returning an HTTP 200 from the API, rather than a 403. i.e. they were expected to fail due to assignments being overdue, but the failure did not occur.

• test_submit_after_deadline
• test_group_submit_with_extension
• test_submit_with_expired_extension
• test_submit_with_extension

These all failed with the message AssertionError: 200 != 403 : HTTP Status 403 expected but got 200. If you look through the Travis builds for OK.py you can see some of these tests failing, at times, during the Travis build.

When running the tests two Assignments are created, and then used when testing the API. Prior to this PR, these Assignments used datetime.now as the basis of the due date and the lock date for the assignment. I.e. the code looked like this...

due_date=dt.datetime.now(),
lock_date=dt.datetime.now() + dt.timedelta(days=1),

These two dates are then inspected when various API calls are made. Within the API there are lines of code such as this...

past_due = dt.utcnow() > assignment['due_date']

... and this property exists on the Assignment...

def active(self):
        return dt.datetime.utcnow() <= self.lock_date

The above comparisons use utcnow.

Let's take an example of what this fixes. Assume that the unit tests all run at exactly 18:00 UTC on a given date. The tests that I see failing all reduce the due_date by 1 hour. So, let's look at how the unchanged code would then function in BST (British Summer Time, UTC+1), GMT (Greenwich Mean Time, UTC+0) and PDT (Pacific Daylight Time, UTC-7)

Test Time (UTC)	due_date (BST)	due_date in GMT	due_date in PDT
18:00	18:00	17:00	10:00

As you can see, if you're looking at the past_due code above, this will resolve to:

• TRUE (the submission is overdue) in PDT, because the server compares 18:00 with a due_date of 11:00
• TRUE (the submission is overdue) in GMT, because the server compares 18:00 with a due_date of 17:00...but, the lock_date for this submission will be 17:00 and so the run time performance of the tests might come into play.
• TRUE or FALSE in BST, because the server compares 18:00 with the due_date of 18:00, and so the run time performance of the tests might come into play.

All of which is to say, in a rather long-winded manner, that these 4 changed line stop my tests failing, and I think will prevent intermittent failures in Docker where the local time == UTC.

Longer term, I think timezone needs to play a more explicit role in the way Assignments are given due dates and lock dates.

[martinpeck]

Hmmm....Travis CI builds are failing because of a different time-based unit test. Namely, test_submit_with_expired_extension.

[martinpeck]

Another timezone dependant test. I've played around with setting the timezone within the Docker image. Doing this can cause some of the tests to fail due to Assignments being "open" when they should be over due/locked (i.e. the fail with AssertionError: 200 != 403 : HTTP Status 403 expected but got 200)

For example...

1. Put this into Dockerfile and all tests pass (YAY!):

ENV TZ=Etc/UTC
RUN ln -snf /usr/share/zoneinfo/$TZ /etc/localtime && echo $TZ > /etc/timezone

2. Put this into Dockerfile and I see two tests fail (test_submit_after_deadline and test_submit_with_extension):

ENV TZ=America/Los_Angeles
RUN ln -snf /usr/share/zoneinfo/$TZ /etc/localtime && echo $TZ > /etc/timezone

3. Put this into Dockerfile and I see two tests fail (test_submit_with_expired_extension and test_submit_with_extension):

ENV TZ=America/New_York
RUN ln -snf /usr/share/zoneinfo/$TZ /etc/localtime && echo $TZ > /etc/timezone

Put this into Dockerfile and I see three tests fail (test_submit_after_deadline, test_submit_with_expired_extension and test_submit_with_extension):

ENV TZ=US/Central
RUN ln -snf /usr/share/zoneinfo/$TZ /etc/localtime && echo $TZ > /etc/timezone

[Sumukh]

Thanks for looking into this.

Longer term, I think timezone needs to play a more explicit role in the way Assignments are given due dates and lock dates.

I agree.

[colinschoen]

@martinpeck Thanks for working on this. Should the following be added to Dockerfile then?

ENV TZ=Etc/UTC RUN ln -snf /usr/share/zoneinfo/$TZ /etc/localtime && echo $TZ > /etc/timezone

[martinpeck]

If you add this to the Dockerfile then it means the tests pass....but I'm not sure that's a good thing. It's not clear, to me, if that just masks the problem. However, if you want to passing tests this I can add this.

What do you think? Is it better to get them passing in UTC for now?

[colinschoen]

Yeah let’s add that for now. It should be pretty rare for servers not to be running UTC. I’ll look into this more.