Travis ci #308

shubhamshuklaer · 2016-05-28T14:59:53Z

~~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.

Now `ghi milestone sample_title -m sample_description` will create a milestone with title as sample_title and description as sample_description. Fixes stephencelis#303

shubhamshuklaer · 2016-05-28T17:03:17Z

README.md

@@ -1,5 +1,7 @@
 # ghi

+[![Build Status](https://travis-ci.org/shubhamshuklaer/ghi.svg?branch=travis-ci)](https://travis-ci.org/shubhamshuklaer/ghi)


The link needs to be changed after merge.

davidCarlos · 2016-05-30T03:41:16Z

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 · 2016-05-30T04:24:28Z

@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 · 2016-05-30T04:37:01Z

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 · 2016-05-30T04:41:35Z

@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 · 2016-05-30T05:47:14Z

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 travis-ci/travis-ci#6112)

shubhamshuklaer · 2016-06-17T02:01:47Z

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 · 2016-06-17T09:18:11Z

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~~

Helpful while doing local testing. We don't want to mess with the user's original ~/.gitconfig. User will want to use a different github account for testing(Don't want to mess up the original account).

stephencelis · 2016-06-18T15:44:41Z

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 · 2016-06-18T16:39:09Z

.editorconfig

+trim_trailing_whitespace = true
+
+[*.rb]
+indent_style = tab


Looks like the project uses space instead of tabs. I will fix this.

shubhamshuklaer · 2016-06-18T17:10:39Z

@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 · 2016-06-18T17:21:03Z

The reason for keeping the username and password secure(Not available publically) even though its a fake account.
shubhamshuklaer#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 · 2016-07-03T22:20:52Z

@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 · 2016-07-13T07:54:33Z

@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 · 2016-12-08T14:46:22Z

README.md


 ![Example](images/example.png)
+
+## Testing Guidlines


typo: Guidlines => Guidelines

olleolleolle · 2016-12-08T14:47:11Z

README.md

+* Before running tests `GITHUB_USER` and `GITHUB_PASSWORD` environment
+variables must be exported. It is best to use a fake account as a bug can mess
+up your original account. You can either export these 2 environment variables 
+through `~/.bashrc`(As ghi only uses these while generating its token, so after


Space missing before (As

olleolleolle · 2016-12-08T14:47:29Z

README.md

+variables must be exported. It is best to use a fake account as a bug can mess
+up your original account. You can either export these 2 environment variables 
+through `~/.bashrc`(As ghi only uses these while generating its token, so after
+you generate the token for your original account(for regular use), fake account


Space missing before (for

olleolleolle · 2016-12-08T14:48:29Z

README.md

+* Open a Travis CI account and activate travis-ci for the fork
+* Create a fake github account for testing. The username, password and token
+will be available to the tests and if by mistake the test prints it, it will be
+available in public log. So its best to create a fake account and use a


typo its => it's

olleolleolle · 2016-12-08T14:49:04Z

README.md

+* At Travis-CI, on the settings page for the fork, add environment variables
+`GITHUB_USER` and `GITHUB_PASSWORD`. Ensure that the "Display value in build
+log" is set to false. It is possible to add these in ".travis.yml", but don't
+as all forks as well as original repo will be using different accounts(We cannot 


Space missing before (We

olleolleolle · 2016-12-08T14:49:57Z

lib/ghi/commands/config.rb

          opts.on '--local', 'set for local repo only' do
            assigns[:local] = true
          end
+          opts.on '--just_print_token', "Just print the token don't add it to ~/.gitconfig(Useful while testing)" do


Space missing before (Useful

olleolleolle · 2016-12-08T14:50:58Z

test/assign_test.rb

@@ -0,0 +1,42 @@
+require "test/unit"
+require "helper"
+require "pp"


pp no longer used in this file.

olleolleolle · 2016-12-08T14:51:14Z

test/close_test.rb

@@ -0,0 +1,30 @@
+require "test/unit"
+require "helper"
+require "pp"


pp no longer used in this file.

olleolleolle · 2016-12-08T14:51:39Z

test/comment_test.rb

@@ -0,0 +1,45 @@
+require "test/unit"
+require "helper"
+require "pp"


pp not used.

olleolleolle · 2016-12-08T14:52:07Z

test/delete_all_repos.rb

@@ -0,0 +1,30 @@
+require "helper"
+require "json"
+require "pp"


pp not used.

olleolleolle · 2016-12-08T14:54:31Z

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

shubhamshuklaer · 2016-12-09T16:47:59Z

@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

shubhamshuklaer added 2 commits May 21, 2016 18:08

Take title and description on cmdline for milestone just like open

5c7a93d

Now `ghi milestone sample_title -m sample_description` will create a milestone with title as sample_title and description as sample_description. Fixes stephencelis#303

Take title and description on cmdline for edit just like open

8c0b64d

shubhamshuklaer reviewed May 28, 2016
View reviewed changes

shubhamshuklaer mentioned this pull request May 29, 2016

Add support for pull requests #288

Open

Add .editorconfig to maintain project's coding style

3dde9eb

shubhamshuklaer force-pushed the travis-ci branch from 99fd4a9 to bee0f7a Compare June 17, 2016 13:22

shubhamshuklaer added 16 commits June 17, 2016 19:11

Setup framework for travis-ci and local testing

b55c072

Add --just_print_token option to ghi config --auth

143c310

Helpful while doing local testing. We don't want to mess with the user's original ~/.gitconfig. User will want to use a different github account for testing(Don't want to mess up the original account).

Setup communication with github

e5e4e14

Setup mock data and its helper functions

07db9a8

Setup repo and token creation/deletion

6d4e230

Test opening issue

8a18d52

Test comment creation/ammend/deletion

cb45d58

Test milestone creation

91e338f

Test [un-]assigning user to the issue

082c583

Test editing issue

1309950

Test adding/deleting/replacing labels from issue

17154f8

Test closing issue

985c3f2

Test showing issue

614be1d

Test listing issue

7a336de

A helper script for cleaning up(deleting all repos)

ed7086a

Update Readme

afb75ef

shubhamshuklaer force-pushed the travis-ci branch from c389749 to afb75ef Compare June 17, 2016 13:42

shubhamshuklaer reviewed Jun 18, 2016
View reviewed changes

Convert tabs to spaces. The project uses spaces

072bd1e

shubhamshuklaer changed the title ~~WIP: Travis ci~~ Travis ci Jun 18, 2016