Skip to content
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.

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

Memory and file tests #745

Merged
merged 10 commits into from
Jun 3, 2017
Merged

Memory and file tests #745

merged 10 commits into from
Jun 3, 2017

Conversation

davizucon
Copy link
Collaborator

This will address this #723

@davizucon
Copy link
Collaborator Author

Can someone help ?
https://travis-ci.org/gunthercox/ChatterBot/jobs/233244184

Warning, treated as error:
/home/travis/build/gunthercox/ChatterBot/docs/sessions.rst.rst:39:Over dedent has detected

@davizucon davizucon requested a review from gunthercox May 17, 2017 14:13
@vkosuri
Copy link
Collaborator

vkosuri commented May 17, 2017

Here two issues are there one is pep8 issue

Expected single line but it has multiple lines

./tests/storage_adapter_tests/test_sqlalchemy_adapter.py:402:1: E303 too many blank lines (3)
./chatterbot/storage/sqlalchemy_storage.py:16:5: E303 too many blank lines (2)
./chatterbot/storage/sqlalchemy_storage.py:41:5: E303 too many blank lines (2)

Another issue dedent issue http://www.sphinx-doc.org/en/stable/markup/code.html#dedent

You should specify line number also here

.. literalinclude:: ../chatterbot/ext/django_chatterbot/views.py
   :language: python
   :pyobject: ChatterBotView.post
   :dedent: 4

The it should be like this

.. literalinclude:: ../chatterbot/ext/django_chatterbot/views.py
   :language: python
   :pyobject: ChatterBotView.post
   :dedent: 4
   :lines: 31-32

@davizucon
Copy link
Collaborator Author

@vkosuri thanks, but I don't know why now build is failing by sphinx documentation in classes that I not touched. :(

@vkosuri
Copy link
Collaborator

vkosuri commented May 18, 2017

Not sure, A similar issue found here also sphinx-doc/sphinx#3755

@vkosuri
Copy link
Collaborator

vkosuri commented May 23, 2017

It's sphinx bug fixed in this revision sphinx-doc/sphinx@dabd356

@davizucon
Copy link
Collaborator Author

Great news! Maybe need to wait Sphinx v1.6.2 be released.

Copy link
Owner

@gunthercox gunthercox left a comment

Choose a reason for hiding this comment

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

Sorry about the delay in reviewing this. I've been away for the past week.

I will look into the sphinx issues. Feel free to ignore any failures related to them in this pull request.

def setUp(self):
"""
Make the adapter writable before every test.
Make the adapter read only before every test.
Copy link
Owner

Choose a reason for hiding this comment

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

I don't believe this change to the comment is correct. It would be making it read only if it was setting read_only = True, however the line below is self.adapter.read_only = False.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed.

@davizucon
Copy link
Collaborator Author

Hi @gunthercox , sorry, I got stuck.

@gunthercox
Copy link
Owner

@davizucon I might add some changes to this pull request later today. Is it alright if I work on it?

@davizucon
Copy link
Collaborator Author

@gunthercox Absolutely, feel free.

If someone puts spaces in their database name it should fail.
This is their fault and they deserve to get an error.
@gunthercox gunthercox dismissed their stale review June 3, 2017 15:12

Changes have been addressed.

@gunthercox gunthercox merged commit 10dbc94 into master Jun 3, 2017
@gunthercox gunthercox deleted the sql-memtest branch June 3, 2017 15:12
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.

3 participants