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

Reduce code redundancy in backend testing #4829

Closed
giritheja opened this issue Mar 25, 2018 · 10 comments · Fixed by #5838
Closed

Reduce code redundancy in backend testing #4829

giritheja opened this issue Mar 25, 2018 · 10 comments · Fixed by #5838
Assignees
Labels

Comments

@giritheja
Copy link
Contributor

giritheja commented Mar 25, 2018

In the codebase, the methods get_json and testapp.get are used for the same purpose. In the following screenshot of editor_test.py, get_json is used to verify the successful case (response status = 200) and testapp.get is used to verify the error case (response status != 200)
screen shot 2018-03-25 at 2 52 18 am

The goal of this issue is to replace testapp.get method in all the _test files with get_json method.
Reference: #4574

@seanlip
Copy link
Member

seanlip commented Mar 25, 2018

Hi @giritheja -- do you think you could include a list of files with testapp.get() etc. references that need to be changed? Just wanted to get a sense of how many there are -- hopefully it's small and not really widespread.

Thanks!

@lilithxxx
Copy link
Contributor

Should I take this issue?

@seanlip
Copy link
Member

seanlip commented Oct 9, 2018

Sure, go ahead! Feel free to assign yourself if you'd like to take it up.

@lilithxxx lilithxxx self-assigned this Oct 9, 2018
@lilithxxx
Copy link
Contributor

@seanlip just a question. There are many instances of testapp.get where it fetches html instead of json hence assertion fails. In that case can we still use get_json there? Or should I just change the code where only it fetches json?

@seanlip
Copy link
Member

seanlip commented Oct 10, 2018

That's a good question. I think we should define a self.get_html() for parity with self.get_json(), and then use either of these as appropriate. What do you think?

@lilithxxx
Copy link
Contributor

Yes I agree

@nithusha21
Copy link
Contributor

Hi @lilithxxx any updates on this issue?

@lilithxxx
Copy link
Contributor

@nithusha21 I will surely get into this next week as currently I am travelling

@DubeySandeep
Copy link
Member

@lilithxxx, any update on this?

@lilithxxx
Copy link
Contributor

@DubeySandeep working on this

kevinlee12 pushed a commit that referenced this issue Jan 6, 2019
* Replaced testapp.get with appropriate functions

* Add test cases

* Function get_response

* Function get_response_without_checking_for_errors

* Fixed Errors

* Fixed lint checks

* removed further redundancy

* Lint checks

* Lint checks

* made params only dict type

* Minor changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants