Skip to content

Conversation

@rlr
Copy link
Contributor

@rlr rlr commented Apr 5, 2012

yay! r?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just moved these out of __init__.py to their own file

@willkg
Copy link
Member

willkg commented Apr 6, 2012

Hrm... I think I'll try to work through this later today.

This is pretty intense. I didn't think about the peer-review work of changing tests to use fixtures.

@rlr
Copy link
Contributor Author

rlr commented Apr 6, 2012

Yeah, going forward we might want to do these in more reasonable chunks. Something like ~20 tests a week each or whatever it takes to be done in the quarter.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this covers the test right. It seems like the test is checking to see if for a given forum, the threads are sorted by datetime. But if there is only one thread, then it's always sorted.

Seems like we want to create a forum here. Then create a few threads with various datetimes (probably want to specify the datetimes since it's part of the test), then make sure the ThreadsFeed puts them in order correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

definitely, should be more than one thread :-)

@willkg
Copy link
Member

willkg commented Apr 9, 2012

Yay I finished!

I don't even know how many issues there were. I think some of them were benign and some were about the test itself. The latter are worth fixing. The former if you want.

@rlr
Copy link
Contributor Author

rlr commented Apr 9, 2012

OK, I think that covers it ^^

@willkg
Copy link
Member

willkg commented Apr 10, 2012

I think that looks good to me. r+

@rlr
Copy link
Contributor Author

rlr commented Apr 10, 2012

4478787

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants