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

fix error when handle bad command. #49

Merged
merged 7 commits into from
Jul 28, 2018
Merged

Conversation

tgly307
Copy link
Contributor

@tgly307 tgly307 commented Jul 28, 2018

No description provided.

@codeclimate
Copy link

codeclimate bot commented Jul 28, 2018

Code Climate has analyzed commit 1f26132 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 0.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 54.5% (0.0% change).

View more on Code Climate.

@attzonko attzonko merged commit 37e134d into attzonko:master Jul 28, 2018
@attzonko
Copy link
Owner

Thanks for finding and fixing this. Is there some test we can add to catch stuff like this in the future?

@tgly307
Copy link
Contributor Author

tgly307 commented Jul 29, 2018

In fact, I am not good at writing test cases, I feel that the existing test cases are comprehensive enough. Perhaps due to the historical reasons of this project, some code is still in a stage that is not fully understood, and we can gradually comb the old code to add unit tests. After all, this project does not exist as commercial software, we can slowly find the problem. XD

@attzonko
Copy link
Owner

Totally agree, was just wondering if this issue might be an easy one to add a test case for, and since you hit it and fixed it you might be the best to answer that question. I'll have to think more about your Python version question. I was hoping that we can continue to support both, meaning to be backwards compatible if possible.

@seLain
Copy link
Collaborator

seLain commented Jul 29, 2018

@tgly307 Could you share us more how you bumped into this error ? There might be clue for constructing adequate test cases.

@tgly307
Copy link
Contributor Author

tgly307 commented Jul 30, 2018

@seLain I want to modify default response content, before I start this work, I tried to entered a wrong command to see how this function work.

@tgly307
Copy link
Contributor Author

tgly307 commented Jul 30, 2018

@attzonko OK, got it. But the execfile() function is not support in python 3, although this code will rarely execute,I think it needs to be version compatible here to avoid error like this issue .

@attzonko attzonko added the bug Something isn't working label Jul 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants