Global topics #18
Global topics #18
Conversation
|
Nice, will look into it soon as I get some time. At quick glance, it looks really nice. |
|
Changes commited, should I make another pull request? Nevermind, saw that changes are visible :) |
tests/integration/test_extension.py
Outdated
| topic = self.app.phpbb3.fetch_global_topics( | ||
| skip=skip, | ||
| limit=1, | ||
| forum_id=0) | ||
| self.assertEqual((skip, topic), expected_topics[skip]) | ||
| if skip > 3: |
There was a problem hiding this comment.
I was wondering what this was for.
You can compare lists without any problems.
So, if database has positive and negative items, you only compare if actual result is equal to expected result (only positive).
There was a problem hiding this comment.
Oh, must have misunderstood the the positive vs. negative tests. All tests after index 3 are expected to not be equal.
There was a problem hiding this comment.
Yes, I see that.
What I would like to see is, that this would be more explicitly be seen from code. This does its job, but I needed cca 30 minutes longer to get the point.
By properly separating tests and naming them, you can really make the tests easy readable.
There was a problem hiding this comment.
Ok, I see what you mean, will submit new changes soon.
tests/integration/base.py
Outdated
| ",topic_first_poster_name" | ||
| ",topic_first_post_id" | ||
| ",topic_type" | ||
| ") values (" |
There was a problem hiding this comment.
Whitespace in strings are missing (simulating you would be writing into a file).
Also, I see you could congest each row into one line. While keeping field list with each column in its own line (for readability).
Example:
"insert into phpbb_topics ("
" topic_id"
" ,forum_id"
" ,topic_title"
" ,topic_time"
" ,topic_first_poster_name"
" ,topic_first_post_id"
" ,topic_type"
") values"
"(0,0,'topic title 0',10",'name'",0,3)"
", (1, 0, topic title 1', 13, 'second poster', 1, 3)"
", (2, 0, 'topic title 2', 200, 'post it', 2, 3)"
...
tests/integration/base.py
Outdated
| ",999" | ||
| ",'inserted into table, expecting different type'" | ||
| ",3" | ||
| ",3" |
There was a problem hiding this comment.
Should this number be something else (topic_type)?
I see in your dataset you have only topic_type 3.
There was a problem hiding this comment.
I have set up one example where topic_type is different from 3. But the select expression for global topics has a where clause with a condition that topic_type = 3. So I think it would make sense to have most of examples fulfilling this condition.
| " (1,'druga tema','blabla')," | ||
| " (2,'tretja tema','blablabla')," | ||
| " (3,'tretja tema','bla krat 4')" | ||
| " insert into phpbb_posts (" |
There was a problem hiding this comment.
Be careful!
You have INNER JOIN in the query. Which means other topics won't show up, because their posts do not exist!
Add posts before changing your tests to see if it will break your tests.
There was a problem hiding this comment.
Posts have been added in the next commit.
Added tables topics and posts. Added functionality to fetch global topics.