Travis ci

[shubhamshuklaer]

~~This still needs some work, So don't merge yet.~~This can be merged This contains 2 commits from #305 . Once that is merged I will rebase it off of master. Fixes #302

For a working version check https://github.com/shubhamshuklaer/ghi/tree/travis-ci https://travis-ci.org/shubhamshuklaer/ghi

For merging check https://github.com/shubhamshuklaer/ghi/tree/travis-ci#enable-travis-ci-in-fork

I am almost done, but I wanted to get suggestions from others. If anyone have any idea on how to improve it, please comment. Right now there are only the very basic tests. I won't be able to add any more tests myself(as time ran out), but if anyone wants to contribute they can open a pull request on travis-ci branch on https://github.com/shubhamshuklaer/ghi or wait for this to get merged and then open a pull request on this repo.

Because of the nature of the software, the tests overlap with each other. For eg. for testing ghi edit you first need to create an issue, maybe a milestone too and then edit that issue. As a result editing issue also tests issue creation and milestone creation. But I have also kept separate tests for issue creation and milestone creation, so that they can be tested separately.

[davidCarlos]

Nice job @shubhamshuklaer, i will take a look and try to contribute some how. I think that we could squash some of those commits, cause will be difficult to review all this code with so much commits.

[shubhamshuklaer]

@davidCarlos thanks. You are right. I will reduce the no of commits. Right now I am working on improving local testing. Since the tester will use a fake github account for testing and a genuine account for using ghi, this will cause clash. I was thinking of suggesting the testers to use a different user(computer user) for testing(he can switch user in terminal before running tests) and then that user will have different ~/.gitconfig and different env vars values for GITHUB_USER and GITHUB_PASSWORD. Do you have a better way??

[shubhamshuklaer]

One more thing I will fix is the indentation style. It doesn't match the rest of the project. I will fix it before squashing commits.

[shubhamshuklaer]

@davidCarlos can also evaluate whether the information in readme is enough to setup for local testing as well as setting up travis-ci for fork.

[shubhamshuklaer]

One problem is that the secure(We marked display these in build log to false) env varriables GITHUB_USER and GITHUB_PASSWORD are not provided by travis while building pull requests from fork. The explanation is given here https://docs.travis-ci.com/user/pull-requests#Security-Restrictions-when-testing-Pull-Requests . Hence tests cannot run for pull requests from forks(It will run when the pull request is from the same repo).

Any suggestions for solving it?? One way is to not make it secure. But then it will be public and anyone will be able to see it.

They do give an option to say which tests to run on pull req. from fork but right now all tests need that secure ENV vars.

It would be awesome if they give some button for the admin to click after he examines the pull request for any malicious(or otherwise) test.( I opened an issue for it https://github.com/travis-ci/travis-ci/issues/6112)

[shubhamshuklaer]

One more thing that needs to be fixed is deleting the tokens once done with the tests(We can add a flag if the user don't want to delete it). Since travis spins up different containers on each run(of test suit), each run gets a new token and a lot of tokens are accumulated in github. Its best to delete them(For security reasons as well as management). Right now the token is generated before running the first test. And its not deleted cause we don't know which is the last test. The way to fix it, will be to generate the token before each test and to delete it after each test. This will also make local testing easier, as the user won't have to worry about his ~/.gitconfig , the GITHUB_USER and GITHUB_PASSWORD can be passed using rake test:one_by_one GITHUB_USER='abc' GITHUB_PASSWORD='xyz'

I will work on this today.

[shubhamshuklaer]

Fixed the following things

• Corrected the Indentation style of tests(Now following project's style)
• Improved local testing(Tokens are not added nor used from ~/.gitconfig or global GHI_TOKEN while testing) so you can both test with fake account and use ghi normally with original account simultaneously. While testing token is assigned to GHI_TOKEN env variable of the sub-shell running ghi. ghi runs as a subprocess, so your terminal's env vars are not affected.
• Deleting tokens after running tests(To prevent hundreds of token being left stray in the account)
• Restructured the commits.

TODO

• ~~squash commits~~

[stephencelis]

This is great! And what an undertaking! If it's complete (less the README link), I wonder if @AlexChesters has time to look it over and connect it to the PR mechanism here (to ensure future PRs don't break the build).

[shubhamshuklaer]

@stephencelis Thanks. Its complete from my side(less the README link). About PR mechanism, (as mentioned in a previous comment) since the test environment contains 2 secure env vars namely GITHUB_USER and GITHUB_PASSWORD , travis doesn't provide them to pull request from forks(It considers them insecure). So the test won't work like in other repos where they automatically run tests as soon as the PR is made. However we can run tests manually before merging by pulling the changes to a temporary local branch and then pushing it to github(then the travis will run on that branch) or run the tests locally using rake test:one_by_one GITHUB_USER='xyz' GITHUB_PASSWORD='abc'.

[shubhamshuklaer]

The reason for keeping the username and password secure(Not available publically) even though its a fake account. https://github.com/shubhamshuklaer/ghi/issues/3

Even if the user is fake, it still linked to the creator and if we make username and password public someone can misuse it. For eg 1. uploading bad things to that account 2. run a script which will continuously keep making api request to it, forcing github people to suspend the account. 3. changing the password(Just for causing problems). 4. Using that account to post bad things everywhere(Other's repos) on github.

Even if we encrypt the username and password, it will be de-crypted before running tests, so anyone can write a test which will print it and run that test locally.

[stephencelis]

@shubhamshuklaer Been thinking on this. One possibility to avoid the need to run with an actual user/password would be to use the VCR gem to record the API requests:

https://github.com/vcr/vcr

[shubhamshuklaer]

@stephencelis That is an awesome idea. But I don't know when I will be able to do it.

If someone else is interested in doing it, that would be awesome.

[olleolleolle]

@shubhamshuklaer I wanted to say thanks ✨ by adding a slew of specific notes in the code. Thanks!

[shubhamshuklaer]

@olleolleolle Thanks a lot for the review. I am a bit out of touch with this PR right now. Will fix this when incorporating stephencelis' idea of using vcr gem. Thanks