ghi icon indicating copy to clipboard operation
ghi copied to clipboard

Tests

Open davidCarlos opened this issue 8 years ago • 11 comments

Hey guys, i would like to know why ghi not have a automated test suite.

Aside all the positive arguments about implement tests, It would be more easy, in my opinion, to contribute to ghi, if it had a good test suite.

The fact that ghi interact with github api, can be a problem without tests. If the api changes, ghi will crash, and we will only know using the gem. With tests this kind of thing would be more easy to fix.

davidCarlos avatar May 18 '16 17:05 davidCarlos

I started working on it few days back (https://github.com/shubhamshuklaer/ghi/tree/travis-ci), though I stopped for now. There are few problems..

  1. All the code needed to talk to ghithub, needs to be re-written(Edit: for use in tests) or re-used which won't make for good tests. A good test should be as simple as possible.. so that there is no bug in test. A bad test is worse than no test.
  2. Formatting is also a problem. To test weather the commands work you need to compare the output. But there is a lot for formatting like colors, ticks, pull request(arrow) . That code also needs to be re-implemented or reused.
  3. A lot of things use an editor, to test that part you need to re-implement or reuse the code used to talk to the editor.

About github api changing(I don't know enough), but I don't think they would do that without warning, because a lot of softwares will break.

I agree with the other point about it being easier to contribute if there is a good test suit. Maintenance will also become easier.

shubhamshuklaer avatar May 20 '16 05:05 shubhamshuklaer

They won't change the GitHub API in a breaking way as they follow the pattern of semantic versioning.

https://developer.github.com/v3/versions/

Sent from my iPhone

On 20 May 2016, at 06:50, Shubham Shukla <[email protected]mailto:[email protected]> wrote:

I started working on it few days back (https://github.com/shubhamshuklaer/ghi/tree/travis-ci), though I stopped for now. There are few problems..

  1. All the code needed to talk to ghithub, needs to be re-written or re-used which won't make for good tests. A good test should be as simple as possible.. so that there is no bug in test. A bad test is worse than no test.
  2. Formatting is also a problem. To test weather the commands work you need to compare the output. But there is a lot for formatting like colors, ticks, pull request(arrow) . That code also needs to be re-implemented or reused.
  3. A lot of things use an editor, to test that part you need to re-implement or reuse the code used to talk to the editor.

About github api changing(I don't know enough), but I don't think they would do that without warning, because a lot of softwares will break.

I agree with the other point about it being easier to contribute if there is a good test suit. Maintenance will also become easier.

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHubhttps://github.com/stephencelis/ghi/issues/302#issuecomment-220522923


http://www.bbc.co.uk This e-mail (and any attachments) is confidential and may contain personal views which are not the views of the BBC unless specifically stated. If you have received it in error, please delete it from your system. Do not use, copy or disclose the information in any way nor act in reliance on it and notify the sender immediately. Please note that the BBC monitors e-mails sent or received. Further communication will signify your consent to this.


AlexChesters avatar May 20 '16 06:05 AlexChesters

Nice that gihub api follows semantic versioning, i didn't know it, but the api was just a example. @shubhamshuklaer Did you mapped what in ghi can be easily tested? I think we could start from this point.

davidCarlos avatar May 20 '16 13:05 davidCarlos

@shubhamshuklaer you mention this in your comment

All the code needed to talk to ghithub, needs to be re-written

Are you suggesting ghi should be re-written so that it doesn't talk to GitHub? How would this work?

AlexChesters avatar May 20 '16 13:05 AlexChesters

@AlexChesters No. I meant that if we have to test for eg. creating an issue, we will have to talk to github to test whether the issue was correctly created. Likewise for other tests will also need to talk to github. For talking to github in tests we either have to reuse the ghi's code(client.rb) or write separate code, both of which isn't a good idea. A good test should be independent and simple.

shubhamshuklaer avatar May 20 '16 14:05 shubhamshuklaer

@davidCarlos Well I don't think that there will be any major difficulty. What I was trying to say that we have to make some decisions for how much code to reuse/re-write(Both of which are not good testing practice) or if there is another way(please comment if anyone have suggestions). For eg if we take care of first problem, then everything except "list" and "show" will be easy to test. If we also take care of second problem then "list" and "show" will be done too. If we take care of the third problem then we can test the commands which utilize the editor(All commands also have non editor version too where we can just give arguments on command line).

shubhamshuklaer avatar May 20 '16 15:05 shubhamshuklaer

I started working on this again. I will try to implement basic tests first.

shubhamshuklaer avatar May 21 '16 08:05 shubhamshuklaer

All the code needed to talk to ghithub, needs to be re-written(Edit: for use in tests) or re-used which won't make for good tests

All the commands that only requests data from github (list, show, milestones, status) can be easily mock(a good article to start reading Mocks arent Stubs). We can mock/stubs the response from github for those commands that only make get requests, without having to access github's api directly. With the code that send post / put requests to github i think we can test the response of the Requisitions.For eg, If the response of the endpoint that create issues in the github, return 200, ghi is working as expected (i do not know what github returns when a request is well done, aside the http status). To send this request to github i think we could use the client.rb, and stubs all methods That We judge Necessary to isolate each test. If the test is to hard to implement, my little experience tell me that the source code can be refactored.

With formatting we can mock all code that does not have relation directly with what we are testing. Obviously, use mock techniques will not solve all our problems, but can make our lives easy to test ghi code.

davidCarlos avatar May 24 '16 14:05 davidCarlos

I started working on this again. I will try to implement basic tests first.

I would like to help you on this.

davidCarlos avatar May 24 '16 19:05 davidCarlos

@davidCarlos I found this library https://github.com/typhoeus/typhoeus and this has made life easier(Only had to write around 30 lines for talking to github). I have written test for token generation, opening and closing issue, assign/unassign, edit, comment( create, amend, delete), milestone create, milestone add, adding/deleting/replacing labels.

Note: by interface I mean all the application code, except the part that talks to github.

Thanks for the link. Its very good.

But I think mock won't help that much(Correct me if I am wrong). The thing is talking to github is the most important thing, even if everything else works perfectly, if we can't talk to github the software is useless. We can separate the 2 parts and test the user interface and talking to github separately but that will only increase work. For eg for creating issue first we will test that the interface calls client.rb with proper(expected, need not be the correct) arguments using mocks and then we will separately test that calling client.rb with the proper(expected, need not be the correct) arguments indeed does create issue.

Mocks will reduce the dependency while testing interface(A good thing). For eg. for testing milestone addition, First I test issue creation, test milestone creating and then test adding the milestone to the issue. But with mock I will only have to test whether its calling client.rb with the correct arguments. But this will just postpone the dependency. While testing client.rb that it does add the milestone to the issue, we will have to create an issue, create a milestone and then add milestone to issue. It can't be avoided. Dependency between tests might be a big deal for bigger projects(huge number of test and huge code base), but here its not that big of a deal(Correct me if I am wrong). We just have to know how to reproduce the problem(The command and environment which causes problem), after that(with the help of error trace and print statements), it just takes 5-10 mins to find the actual bug(Solving might take time.. but that's not what the tests are for).

Mocks will be helpful when testing the editor versions of the commands. We can use a mock object instead of the editor.

For talking to github, I feel using the actual api will be better(Especially now that talking to github takes only around 30 lines). It will have a direct effect. If its working in tests, then its almost(Depends on the coverage of tests) sure to work in real environment and if its not working in test then it will not work in real environment.

Whats your opinion?? As well as @AlexChesters .

I would like to help you on this.

Awesome, I would love to collaborate. The more coverage the better. But give me 3-4 days, right now the code is not stable, I am constantly changing things and then it will create merge conflicts. Once it become stable I will open a "Work in progress" pull request. Then anyone can suggest improvements, discuss strategies or contribute directly by opening a pull request to https://github.com/shubhamshuklaer/ghi on travis-ci branch.

shubhamshuklaer avatar May 26 '16 04:05 shubhamshuklaer

@davidCarlos Though one more benefit of mocks will be that we can test fast. While developing we can just run a specific test(And run the talking to github part in the end only). Right now it will take about 30 secs(On slow connection) to test adding milestone cause it will first create an issue and a milestone and then add issue to milestone. With mocks we can just test wether client.rb is called with correct argument. Testing whether the milestone is actually added on github can be done later.

shubhamshuklaer avatar May 26 '16 04:05 shubhamshuklaer