Skip to content

Global topics #18

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Global topics #18

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
6e4fa10
added global topics
GregsCode12 May 9, 2019
f8be8ad
added_testing_data
GregsCode12 May 16, 2019
418b564
passing unit tests for global topics
GregsCode12 May 18, 2019
842ba9b
added data annotaions for global topics
GregsCode12 May 18, 2019
84e020b
corrections in formatting of code
GregsCode12 May 18, 2019
6a86483
whitespaces and new lines corrections
GregsCode12 May 18, 2019
d444427
more formatting corrections
GregsCode12 May 19, 2019
ff30ded
3.7 build corrections
GregsCode12 May 19, 2019
7c6ad65
Changes based on first review
GregsCode12 May 23, 2019
0aec5df
Changes based on first review
GregsCode12 May 23, 2019
41bd085
Changes for 2.7 build, fixed errors
GregsCode12 May 23, 2019
581508d
last format fix
GregsCode12 May 23, 2019
b7fb25f
indentation fix in base.py
GregsCode12 May 23, 2019
f03e813
changes in tests
GregsCode12 Sep 17, 2019
5dc528b
formatting differences
GregsCode12 Sep 17, 2019
7e8996a
testing psql server availability
GregsCode12 Sep 17, 2019
5451ccc
travis postgres version change
GregsCode12 Sep 18, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 0 additions & 18 deletions tests/fixtures/postgres/global_topics_test_data.sql
View file
Open in desktop

This file was deleted.

100 changes: 87 additions & 13 deletions tests/integration/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,22 +124,96 @@ def _create_session(cursor, session_id, user_id):

def _create_global_topics(cursor):
# type: (psycopg2.extensions.cursor) -> None

cursor.execute(
"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,'naslov_teme_0',10,'ime',0,3),"
" (1,0,'naslov teme 1',13,'drugi poster',1,3),"
" (2,0,'naslov teme 2',200,'post it',2,3),"
" (3,0,'naslov teme 3',256,'posted it',3,3)"
"insert into phpbb_topics ("
"topic_id"
",forum_id"
",topic_title"
",topic_time"
",topic_first_poster_name"
",topic_first_post_id"
",topic_type"
") values ("
Copy link
Member

Choose a reason for hiding this comment

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

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)"
        ...

"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"
"), ("
"3"
",0"
",'topic title 3'"
",256"
",'posted it'"
",3"
",3"
"), ("
"4"
",0"
",'topic title 4'"
",666"
",'posted it again'"
",3"
",3"
") , ("
"6"
",0"
",'topic title 5'"
",777"
",'posted as 7, expecting 6'"
",3"
",3"
") , ("
"7"
",0"
",'different data type'"
",999"
",'inserted into table, expecting different type'"
",3"
",3"
Copy link
Member

Choose a reason for hiding this comment

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

Should this number be something else (topic_type)?

I see in your dataset you have only topic_type 3.

Copy link
Author

Choose a reason for hiding this comment

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

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.

")"
)

cursor.execute(
" insert into "
" phpbb_posts(post_id,post_subject,post_text)"
" values (0,'prva tema', 'bla'),"
" (1,'druga tema','blabla'),"
" (2,'tretja tema','blablabla'),"
" (3,'tretja tema','bla krat 4')"
" insert into phpbb_posts ("
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

Posts have been added in the next commit.

"post_id"
",post_subject"
",post_text"
") values ("
"0"
",'topic one'"
", 'hello'"
"), ("
"1"
",'topic two'"
",'hello world'"
"), ("
"2"
",'topic three'"
",'hello hello'"
"), ("
"3"
",'topic three'"
",'hello times four'"
")"
)


Expand Down
62 changes: 46 additions & 16 deletions tests/integration/test_extension.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,42 +76,72 @@ def test_fetch_global_topics(self):
expected_topics = [(0, [{
'topic_id': 0,
'forum_id': 0,
'topic_title': 'naslov_teme_0',
'topic_title': 'topic title 0',
'topic_time': 10,
'topic_first_poster_name': 'ime',
'post_subject': 'prva tema',
'post_text': 'bla'}]),
'topic_first_poster_name': 'name',
'post_subject': 'topic one',
'post_text': 'hello'}]),

(1, [{
'topic_id': 1,
'forum_id': 0,
'topic_title': 'naslov teme 1',
'topic_title': 'topic title 1',
'topic_time': 13,
'topic_first_poster_name': 'drugi poster',
'post_subject': 'druga tema',
'post_text': 'blabla'}]),
'topic_first_poster_name': 'second poster',
'post_subject': 'topic two',
'post_text': 'hello world'}]),
(2, [{
'topic_id': 2,
'forum_id': 0,
'topic_title': 'naslov teme 2',
'topic_title': 'topic title 2',
'topic_time': 200,
'topic_first_poster_name': 'post it',
'post_subject': 'tretja tema',
'post_text': 'blablabla'}]),
'post_subject': 'topic three',
'post_text': 'hello hello'}]),
(3, [{
'topic_id': 3,
'forum_id': 0,
'topic_title': 'naslov teme 3',
'topic_title': 'topic title 3',
'topic_time': 256,
'topic_first_poster_name': 'posted it',
'post_subject': 'tretja tema',
'post_text': 'bla x4'}])]
for skip in range(0, 3):
'post_subject': 'topic three',
'post_text': 'hello times four'}]),
(4, [{
'topic_id': 4,
'forum_id': 2,
'topic_title': 'topic missing',
'topic_time': 666,
'topic_first_poster_name': 'no forum',
'post_subject': 'topic yes, forum no',
'post_text': 'test case'}]),
(5, [{
'topic_id': 5,
'forum_id': 0,
'topic_title': 'topic missing',
'topic_time': 777,
'topic_first_poster_name': 'not existent forum',
'post_subject': 'topic yes, forum no',
'post_text': 'test case'}]),
(6, [{
'unexpected_column': 5,
'random_column': 0,
'topic_title': 'topic missing',
'topic_time': 777,
'do_not_change_all': 'not existent forum',
'post_subject': 'topic yes, forum no',
'post_text': 'test case'}]),
(7, [1, 2, 3])
]

for skip in range(0, 7):
topic = self.app.phpbb3.fetch_global_topics(
skip=skip,
limit=1,
forum_id=0)
self.assertEqual((skip, topic), expected_topics[skip])
if skip > 3:
Copy link
Member

Choose a reason for hiding this comment

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

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).

Copy link
Author

Choose a reason for hiding this comment

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

Oh, must have misunderstood the the positive vs. negative tests. All tests after index 3 are expected to not be equal.

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, I see what you mean, will submit new changes soon.

self.assertNotEqual((skip, topic), expected_topics[skip])
else:
self.assertEqual((skip, topic), expected_topics[skip])


class TestSession(base.TestWithDatabase):
Expand Down