gouthambs / Flask-Blogging

A Markdown Based Python Blog Engine as a Flask Extension.
http://flask-blogging.readthedocs.org/en/latest/
MIT License
690 stars 160 forks source link

Failing Tests Not Propagated Up #152

Closed gouthambs closed 2 years ago

gouthambs commented 2 years ago

@mar1ad Thanks for looking into Github actions and upgrading to nose2. I noticed that even though some tests fail, they are not getting bubbled up. Did you notice this on your end? Any thoughts?

mar1ad commented 2 years ago

Yes I noticed ;-) It seems there's a bug to fix and it's interesting as well to find out why the unittesting doesn't fail on it. I thought it better to wait for you to comment on the PR, not sure if you liked it.

gouthambs commented 2 years ago

I love the idea of using github actions and also thanks for upgrading ro nose2. Both very useful updates. I actually merged your PR and noticed the issue.

gouthambs commented 2 years ago

@mar1ad let me know what you find.

mar1ad commented 2 years ago

The error:

$ nose2 test.test_views.TestViews.test_editor_post -v
test_editor_post (test.test_views.TestViews) ... ERROR:flask-blogging:list index out of range
Traceback (most recent call last):
  File "/home/marald/git/github/mar1ad/Flask-Blogging/flask_blogging/sqlastorage.py", line 259, in get_post_by_id
    r = self._serialise_posts_and_tags_from_joined_rows(
IndexError: list index out of range
ERROR:flask-blogging:list index out of range
Traceback (most recent call last):
  File "/home/marald/git/github/mar1ad/Flask-Blogging/flask_blogging/sqlastorage.py", line 259, in get_post_by_id
    r = self._serialise_posts_and_tags_from_joined_rows(
IndexError: list index out of range
ok
mar1ad commented 2 years ago

The get_post_by_id function throws 2 exceptions in the test_editor_post test (and lots of other tests using the same function). But the test is still correct, the exceptions pushes the "ok" message onto the next line.

I'm not sure, yet, if there is a bug in the _serialise_posts_and_tags_from_joined_rows class method (which is called from get_post_by_id)

or if this is a result of trying to get a post, which doesn't exist?

gouthambs commented 2 years ago

I accidentally closed this one. @mar1ad just revisiting to see if you still see an issue with this particular test.

mar1ad commented 2 years ago

Hi Goutham,

I revisited this test once more, just to be sure.

test.test_sqlastorage.TestSQLiteStorage.test_delete_post, tries to delete a non-existing post. test.test_sqlastorage.TestSQLiteStorage.test_save_post, tries to get a non-existing post. test.test_views.TestViews.test_editor_post, tries to access a non-existing post, twice.

Therefore mocking the exception logging with the merged PR seems valid to me. It's just cosmetic, because the assertions work as expected.

mar1ad commented 2 years ago

Hi @gouthambs, I think we can close this issue. Unless you still see something I'm missing? And maybe we could start pushing a new version to Pypi? This would enable an upgrade to Flask 2.x

gouthambs commented 2 years ago

@mar1ad thanks. Yes I will close and push a new version.