Add Python and Javascript unit tests

[ethanchewy]

Goal:
Write unit test for Javascript and Python files (https://github.com/ethanchewy/PythonBuddy/blob/master/app.py, https://github.com/ethanchewy/PythonBuddy/blob/master/static/js/javascript.js, and https://github.com/ethanchewy/PythonBuddy/blob/master/static/js/cm-validator-remote.js)

How to do this?

• Python: use unit test python library or Flask testing (http://flask.pocoo.org/docs/0.12/testing/#the-first-test)
• Javascript: use a JS unit test library like Jasmine (https://jasmine.github.io/)

Expected results should be fairly straight forward. Just play around with pylint and error messages and just write example cases with said code and pylint response.

[tsarchghs]

I'd like to work on this. :)

[ethanchewy]

@gjergjk71 thanks let me know if you have any questiosn! For Python, in addition to testing the GET/POST requests with Flask testing, you should test individual functions like format_errors in app.py using Python's unittest (https://www.blog.pythonlibrary.org/2016/07/07/python-3-testing-an-intro-to-unittest/)

[tsarchghs]

	def test_check_code_view(self):
		data = {"text":"print('12312')"}
		response = self.client.post("/check_code",data=data)

returns : TypeError: cannot convert dictionary update sequence element #0 to a sequence

While trying to fix this bug I noticed that evaluate_pylint(text) returns a list and check_code view returns jsonify(evaluate_pylint(text)), trying to jsonify a list results in the error above (not allowed because of security reasons in older browsers) but this error is showing only while running tests, am I missing something?

[ethanchewy]

@gjergjk71 can you copy and paste the error here?

[tsarchghs]

@ethanchewy

ERROR: test_check_code_view (__main__.TestViews)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/test_views.py", line 26, in test_check_code_view
    response = self.client.post("/check_code",data=data)
  File "/usr/local/lib/python3.6/dist-packages/werkzeug/test.py", line 840, in post
    return self.open(*args, **kw)
  File "/usr/local/lib/python3.6/dist-packages/flask/testing.py", line 108, in open
    follow_redirects=follow_redirects)
  File "/usr/local/lib/python3.6/dist-packages/werkzeug/test.py", line 803, in open
    response = self.run_wsgi_app(environ, buffered=buffered)
  File "/usr/local/lib/python3.6/dist-packages/werkzeug/test.py", line 716, in run_wsgi_app
    rv = run_wsgi_app(self.application, environ, buffered=buffered)
  File "/usr/local/lib/python3.6/dist-packages/werkzeug/test.py", line 923, in run_wsgi_app
    app_rv = app(environ, start_response)
  File "/usr/local/lib/python3.6/dist-packages/flask/app.py", line 1836, in __call__
    return self.wsgi_app(environ, start_response)
  File "/usr/local/lib/python3.6/dist-packages/flask_socketio/__init__.py", line 43, in __call__
    start_response)
  File "/usr/local/lib/python3.6/dist-packages/engineio/middleware.py", line 49, in __call__
    return self.wsgi_app(environ, start_response)
  File "/usr/local/lib/python3.6/dist-packages/flask/app.py", line 1820, in wsgi_app
    response = self.make_response(self.handle_exception(e))
  File "/usr/local/lib/python3.6/dist-packages/flask/app.py", line 1403, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/usr/local/lib/python3.6/dist-packages/flask/_compat.py", line 33, in reraise
    raise value
  File "/usr/local/lib/python3.6/dist-packages/flask/app.py", line 1817, in wsgi_app
    response = self.full_dispatch_request()
  File "/usr/local/lib/python3.6/dist-packages/flask/app.py", line 1477, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/usr/local/lib/python3.6/dist-packages/flask/app.py", line 1381, in handle_user_exception
    reraise(exc_type, exc_value, tb)
  File "/usr/local/lib/python3.6/dist-packages/flask/_compat.py", line 33, in reraise
    raise value
  File "/usr/local/lib/python3.6/dist-packages/flask/app.py", line 1475, in full_dispatch_request
    rv = self.dispatch_request()
  File "/usr/local/lib/python3.6/dist-packages/flask/app.py", line 1461, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "tests/../app.py", line 64, in check_code
    return jsonify(output)
  File "/usr/local/lib/python3.6/dist-packages/flask/json.py", line 237, in jsonify
    return current_app.response_class(dumps(dict(*args, **kwargs),
TypeError: cannot convert dictionary update sequence element #0 to a sequence

[ethanchewy]

@gjergjk71 could you open a pull request or copy and paste the code here so that I can debug it out myself?

[dgmouris]

I feel like this would be interesting to work on. Although I feel like doing small piece at a time rather than a gigantic PR would be good.

Although it might need some framework like pytest, but unittest is always great if you just want that. I don't want to impede on work currently being done or architectural choices if it would step on peoples toes.

What do you think? @ethanchewy

[ethanchewy]

I don't think @gjergjk71 is working on this anymore. Feel free to start on this PR and break it down into multiple steps by opening seperate, smaller PR's!

Thanks!

[dgmouris]

Hey @ethanchewy I added pytest just as a runner to simplify the process.

I was wondering if you liked the format of these test: #51

I'm open to feedback if you'd like them differently.

[ethanchewy]

Added comments.

Also, food for thought, can we test the following methods as well?

• evaluate_pylint => can we check that a temp file was created?
• test socket io disconnect() function => can we check the temp file was deleted?

Thanks for all the help and good job!

[ethanchewy]

Also, are you interested in writing the JS unit tests as well? If not, that's totally fine!

[dgmouris]

Hey @ethanchewy thanks for the feedback! This is great, I'm glad you liked the tests.

I agree that we should tests those functions, the one thing that I might do is refactor a bit if you're okay with that:) I find it easier to test smaller functions and I think there's some places that I could break it out a bit more.

I think I might take a hack at that those functions soon, but I think I'm going to make a separate pull request for that so that we can get this part in, so that there's some work done on this in the master branch.

I might take a stab at the JS tests, but not right now:)

PS. I really think this is a cool project, thanks for letting me a part of it.

[ethanchewy]

Sounds good!

[ethanchewy]

@dgmouris Can you check out the work that @chaps did over here: #52, #53

I'll try to combine both of your commits together.

[dgmouris]

Yeah I can take a look at this.

I think we can take a look at this @chaps has the tests for all of the empty pages which is good.

Suggestion for merging, we could merge one (say @chaps PR) then rebase the other branch and merge the other one in (say @dgmouris) we could do this in any particular order. Let me know what you think, as that might be a messy task.

Would you like me to review and add some thoughts to #52, #53 (as we're working on the same thing)?

[ethanchewy]

@dgmouris yeah, that would be great if you could review them!

[dgmouris]

Hey @ethanchewy I tested those methods referenced in the above comment

• evaluate_pylint => can we check that a temp file was created?
• test socket io disconnect() function => can we check the temp file was deleted?

I removed the file delete into its' own function because i couldn't get the "flask_test_client" parameter in the "socket_io.test_client" function to work.

Let me know what you think. I've also added the travis runner, and how to get an html coverage report and added it into the .gitignore.

Sorry its' such a huge PR. but coverage at 92% :)

[dgmouris]

I also check to see that the file contents are changed in the tempfile.

[ethanchewy]

@dgmouris sounds good - thanks so much!

In general, for best practices, you should make your PR as small as possible. So, it's better to have multiple, smaller PR's instead of 1 giant PR. This makes it easier to read and also reference to in the future if we need to roll back the repo for some reason.

Thanks!

[dgmouris]

@ethanchewy I agree, but there was another PR working on the exact same thing... ( #52, #53 ) otherwise I would have made them smaller PRs.

[dgmouris]

I was thinking perhaps of taking a stab at the JS tests for this project, as it's kind of "greenfield" area in this repo. I was thinking of maybe using jest...

I don't want to overstep bounds by making framework decisions on a project, so would like to throw this by you if you have any preference for JS testing frameworks.

PS. sorry for the huge PR:)

[ethanchewy]

@dgmouris that sounds good to me!

Sorry I have to finish a few deadlines for my school commitments but I should be able to review and merge your PR's within the next week!

[dgmouris]

Hey no worries, take your time!