octokit / octokit.rb

Ruby toolkit for the GitHub API
http://octokit.github.io/octokit.rb/
MIT License
3.83k stars 1.13k forks source link

Add missing endpoints to Reactions #1637

Closed wJoenn closed 8 months ago

wJoenn commented 9 months ago

Resolves #1633


Before the change?

After the change?

Pull request checklist

Does this introduce a breaking change?

Please see our docs on breaking changes to help!


I'm having an issue trying to run the tests I've written. I've exported a OCTOKIT_TEST_GITHUB_TOKEN with a new token I've created before forking the app (the token has all the rights I could give it) but despite doing that I get a Octokit::Unauthorized: POST https://api.github.com/user/repos: 401 - Bad credentials error.

I've tried another token that I use in my personal repos and it fails too so I don't think the issue comes from the tokens but probably from something I haven't set up proprelly.

Because of that I haven't been able to assert passing tests and complete coverage yet.

My set up process was to fork the repo then

git clone ...
cd octokit.rb
script/bootstrap
export OCTOKIT_TEST_GITHUB_TOKEN=...
script/test

image

wJoenn commented 9 months ago

@nickfloyd Maybe you'd have some insights to help me finish this PR ? 🙏

kfcampbell commented 8 months ago

Are you able to run the tests on the main branch following those steps? When I run ./script/test on main I get the following output:

===> Bundling...
Please export OCTOKIT_TEST_GITHUB_LOGIN
Please export OCTOKIT_TEST_GITHUB_PASSWORD
Please export OCTOKIT_TEST_GITHUB_TOKEN
Please export OCTOKIT_TEST_GITHUB_CLIENT_ID
Please export OCTOKIT_TEST_GITHUB_CLIENT_SECRET
===> Running specs...
rspec-queue

Randomized with seed 20797

Starting test-queue master (/tmp/test_queue_164308_4020.sock)
/home/kfcampbell/github/dev/octokit.rb/spec/octokit/client/codespaces_secrets_spec.rb:6: warning: method redefined; discarding old create_box
/home/kfcampbell/github/dev/octokit.rb/spec/octokit/client/actions_secrets_spec.rb:6: warning: previous definition of create_box was here
/home/kfcampbell/github/dev/octokit.rb/spec/octokit/client/dependabot_secrets_spec.rb:6: warning: method redefined; discarding old create_box
/home/kfcampbell/github/dev/octokit.rb/spec/octokit/client/codespaces_secrets_spec.rb:6: warning: previous definition of create_box was here

==> Summary (8 workers in 9.7157s)

    [ 1]                                      10 examples, 0 failures         2 suites in 2.4977s      (pid 164311 exit 0 )
    [ 2]                                      88 examples, 0 failures         9 suites in 9.7121s      (pid 164312 exit 0 )
    [ 3]                                     110 examples, 0 failures        11 suites in 9.7050s      (pid 164313 exit 0 )
    [ 4]                                     119 examples, 0 failures        12 suites in 9.6922s      (pid 164314 exit 0 )
    [ 5]                                     109 examples, 0 failures         3 suites in 9.6821s      (pid 164315 exit 0 )
    [ 6]                                     159 examples, 0 failures        19 suites in 9.6698s      (pid 164316 exit 0 )
    [ 7]                                     114 examples, 0 failures         5 suites in 9.6565s      (pid 164317 exit 0 )
    [ 8]                                     195 examples, 0 failures        12 suites in 9.6379s      (pid 164318 exit 0 )

On your branch, I see the errors present in CI. There's a code problem to be addressed here before tests are runnable.

wJoenn commented 8 months ago

Hey @kfcampbell Yeah I'm able to run the test on main just fine, that's because the cassettes already have all the http responses stored in json files so they don't need to make a new one per test.

In the case of my 3 tests though those cassettes don't exist yet so an actual http request need to be made and that's the issue. If I were to take this test for example :

describe Octokit::Client::Reactions do
  before do
    Octokit.reset!
    @client = oauth_client
  end

  context 'with repository', :vcr do
    before(:each) do
      @repo = @client.create_repository('an-repo', auto_init: true)
    end

    after(:each) do
      @client.delete_repository(@repo.full_name)
    rescue Octokit::NotFound
    end

    context 'with release' do
      before do
        @release = @client.create_release(@repo.full_name, 'v1.0.0')
        @reaction = @client.create_release_reaction(@repo.full_name, @release.id, '+1')
      end

      describe '.release_reactions' do
        it 'returns an Array of reactions' do
          reactions = @client.release_reactions(@repo.full_name, @release.id)
          expect(reactions).to be_kind_of Array
          assert_requested :get, github_url("/repos/#{@repo.full_name}/releases/#{@release.id}/reactions")
        end
      end # .release_reactions
    end
  end
end

I get this error

1) Octokit::Client::Reactions with repository with release .delete_release_reaction deletes the reaction
     Got 0 failures and 2 other errors:

     1.1) Failure/Error: raise error

          Octokit::Unauthorized:
            POST https://api.github.com/user/repos: 401 - Bad credentials // See: https://docs.github.com/rest
          # ./lib/octokit/response/raise_error.rb:14:in `on_complete'
          # ./lib/octokit/middleware/follow_redirects.rb:73:in `perform_with_redirection'
          # ./lib/octokit/middleware/follow_redirects.rb:61:in `call'
          # ./lib/octokit/connection.rb:156:in `request'
          # ./lib/octokit/connection.rb:28:in `post'
          # ./lib/octokit/client/repositories.rb:160:in `create_repository'
          # ./spec/octokit/client/reactions_spec.rb:11:in `block (3 levels) in <top (required)>'

     1.2) Failure/Error: @client.delete_repository(@repo.full_name)

          NoMethodError:
            undefined method `full_name' for nil:NilClass
          # ./spec/octokit/client/reactions_spec.rb:15:in `block (3 levels) in <top (required)>'

So the issue is not coming from my code (yet), my code has not even be executed by the test because I'm unable to create the @repo despite doing export OCTOKIT_TEST_GITHUB_TOKEN=<<token>> or export OCTOKIT_TEST_GITHUB_TOKEN=Bearer <<token>> in my terminal before running the tests

kfcampbell commented 8 months ago

I've spent a little while longer looking at this. I'm able to regenerate the spec file by setting the environment variable OCTOKIT_TEST_VCR_RECORD to true and then running bundle exec rspec spec/octokit/client/reactions_spec.rb:148.

Doing this gives me some failure output:

Failures:

  1) Octokit::Client::Reactions with repository with release .delete_release_reaction deletes the reaction
     Failure/Error: assert_requested :delete, github_url("/repos/#{@repo.full_name}/releases/#{@release.id}/reactions/#{@reaction.id}")

       The request DELETE https://api.github.com/repos/kfcampbell/an-repo/releases/127001558/reactions/179987855 was expected to execute 1 time but it executed 0 times

       The following requests were made:

       POST https://api.github.com/user/repos with body '{"auto_init":true,"name":"an-repo"}' with headers {'Accept'=>'application/vnd.github.v3+json', 'Accept-Encoding'=>'gzip;q=1.0,deflate;q=0.6,identity;q=0.3', 'Authorization'=>'token ghp_TOKEN_REDACTED', 'Content-Type'=>'application/json', 'User-Agent'=>'Octokit Ruby Gem 7.2.0'} was made 1 time
       POST https://api.github.com/repos/kfcampbell/an-repo/releases with body '{"tag_name":"v1.0.0"}' with headers {'Accept'=>'application/vnd.github.v4+json', 'Accept-Encoding'=>'gzip;q=1.0,deflate;q=0.6,identity;q=0.3', 'Authorization'=>'token ghp_TOKEN_REDACTED', 'Content-Type'=>'application/json', 'User-Agent'=>'Octokit Ruby Gem 7.2.0'} was made 1 time
       POST https://api.github.com/repos/kfcampbell/an-repo/releases/127001558/reactions with body '{"content":"+1"}' with headers {'Accept'=>'application/vnd.github.v3+json', 'Accept-Encoding'=>'gzip;q=1.0,deflate;q=0.6,identity;q=0.3', 'Authorization'=>'token ghp_TOKEN_REDACTED', 'Content-Type'=>'application/json', 'User-Agent'=>'Octokit Ruby Gem 7.2.0'} was made 1 time
       DELETE https://api.github.com/repos/kfcampbell/an-repo/issues/127001558/reactions/179987855 with body '{}' with headers {'Accept'=>'application/vnd.github.v3+json', 'Accept-Encoding'=>'gzip;q=1.0,deflate;q=0.6,identity;q=0.3', 'Authorization'=>'token ghp_TOKEN_REDACTED', 'Content-Type'=>'application/json', 'User-Agent'=>'Octokit Ruby Gem 7.2.0'} was made 1 time

       ============================================================
     # ./spec/octokit/client/reactions_spec.rb:173:in `block (5 levels) in <top (required)>'

  2) Octokit::Client::Reactions with repository with release .create_release_reaction creates a reaction
     Failure/Error: assert_requested :post, github_url("/repos/#{@repo.full_name}/releases/#{@release.id}/reactions")

       The request POST https://api.github.com/repos/kfcampbell/an-repo/releases/127001568/reactions was expected to execute 1 time but it executed 2 times

       The following requests were made:

       POST https://api.github.com/user/repos with body '{"auto_init":true,"name":"an-repo"}' with headers {'Accept'=>'application/vnd.github.v3+json', 'Accept-Encoding'=>'gzip;q=1.0,deflate;q=0.6,identity;q=0.3', 'Authorization'=>'token ghp_TOKEN_REDACTED', 'Content-Type'=>'application/json', 'User-Agent'=>'Octokit Ruby Gem 7.2.0'} was made 1 time
       POST https://api.github.com/repos/kfcampbell/an-repo/releases with body '{"tag_name":"v1.0.0"}' with headers {'Accept'=>'application/vnd.github.v3+json', 'Accept-Encoding'=>'gzip;q=1.0,deflate;q=0.6,identity;q=0.3', 'Authorization'=>'token ghp_TOKEN_REDACTED', 'Content-Type'=>'application/json', 'User-Agent'=>'Octokit Ruby Gem 7.2.0'} was made 1 time
       POST https://api.github.com/repos/kfcampbell/an-repo/releases/127001568/reactions with body '{"content":"+1"}' with headers {'Accept'=>'application/vnd.github.v3+json', 'Accept-Encoding'=>'gzip;q=1.0,deflate;q=0.6,identity;q=0.3', 'Authorization'=>'token ghp_TOKEN_REDACTED', 'Content-Type'=>'application/json', 'User-Agent'=>'Octokit Ruby Gem 7.2.0'} was made 2 times

       ============================================================
     # ./spec/octokit/client/reactions_spec.rb:166:in `block (5 levels) in <top (required)>'

Finished in 11.35 seconds (files took 0.24878 seconds to load)
3 examples, 2 failures

Failed examples:

rspec ./spec/octokit/client/reactions_spec.rb:171 # Octokit::Client::Reactions with repository with release .delete_release_reaction deletes the reaction
rspec ./spec/octokit/client/reactions_spec.rb:163 # Octokit::Client::Reactions with repository with release .create_release_reaction creates a reaction

Which appears to be relevant. Do you have thoughts on that?

wJoenn commented 8 months ago

Yes, thank you so much @kfcampbell The first error was because I was using the wrong delete method in my DELETE test. delete_issue_reaction instead of delete_release_reaction because I didn't pay enough attention when copying an existing test.

The second error was caused because I was creating another reaction in my before do instead of only inside the DELETE test which is where I needed it. Because of that the POST request was executed 2 times instead of 1.

Both errors are fixed and all tests pass now 🙌

==> Summary (12 workers in 9.3124s)

    [ 1]                                       9 examples, 0 failures         1 suites in 2.4855s      (pid 9391 exit 0 )
    [ 2]                                      20 examples, 0 failures         1 suites in 9.3084s      (pid 9392 exit 0 )
    [ 3]                                      84 examples, 0 failures         8 suites in 9.3080s      (pid 9393 exit 0 )
    [ 4]                                      70 examples, 0 failures         8 suites in 9.3078s      (pid 9394 exit 0 )
    [ 5]                                     148 examples, 0 failures         6 suites in 9.3075s      (pid 9395 exit 0 )
    [ 6]                                      97 examples, 0 failures         5 suites in 9.3071s      (pid 9396 exit 0 )
    [ 7]                                      68 examples, 0 failures         8 suites in 9.3067s      (pid 9397 exit 0 )
    [ 8]                                      96 examples, 0 failures         6 suites in 9.3064s      (pid 9398 exit 0 )
    [ 9]                                      76 examples, 0 failures         9 suites in 9.3059s      (pid 9399 exit 0 )
    [10]                                     111 examples, 0 failures         5 suites in 9.3041s      (pid 9400 exit 0 )
    [11]                                      63 examples, 0 failures         8 suites in 9.3022s      (pid 9401 exit 0 )
    [12]                                      68 examples, 0 failures         8 suites in 9.3015s      (pid 9402 exit 0 )