1

I wrote a GithubClient class which is basically a wrapper for Ruby's Octokit library.

It contains one public method aside from the constructor, called push, which basically creates commit in the remote.

I have made the instance variables parameters to the constructor and then provided defaults, so I can inject a mock.

It has private methods to create commits and push commit trees which uses Octokit, and I think I'm using it in a standard way. I copied most of the code online.

class GithubClient
  def initialize(
    octokit_client=Octokit::Client.new(:access_token => ENV['GITHUB_WRITE_TOKEN']),
    repo_name='main',
    ref_name='heads/master'
  )
    @client = octokit_client
    @repo = repo_name
    @ref = ref_name
  end

  def push(underwriter, files)
    sha_new_commit = create_new_commit(underwriter, files)

    push_commit_tree(sha_new_commit)
  end

  private
  def create_new_commit(underwriter, files)
    sha_latest_commit = @client.ref(@repo, @ref).object.sha
    sha_base_tree = @client.commit(@repo, sha_latest_commit).commit.tree.sha

    new_commit_tree = files.map do |file|
      blob_sha = @client.create_blob(@repo, file)

      {
        :path => file.git_path_name,
        :mode => "100644",
        :type => "blob",
        :sha => blob_sha,
      }
    end

    sha_new_tree = @client.create_tree(@repo, new_commit_tree, {:base_tree => sha_base_tree }).sha
    commit_message = "commit - #{files.first.name}"
    sha_new_commit = @client.create_commit(@repo, commit_message, sha_new_tree, sha_latest_commit).sha
  end

  def push_commit_tree(sha_new_commit)
    @client.update_ref(@repo, @ref, sha_new_commit)
  end
end

Then I wrote a mock test for this, as shown below

  def test_octokit
    octokit_client = mock()
    octokit_client.stubs(:ref).returns(stub(:object => stub(:sha => "sha1")))
    octokit_client.stubs(:commit).returns(stub(:commit => stub(:tree => stub(:sha => "sha2"))))
    octokit_client.stubs(:create_blob).returns("blob sha")
    octokit_client.stubs(:create_commit).returns(stub(:sha => "new commit sha"))

    octokit_client.expects(:create_tree).with("repo_name", [{:path => "", :mode => "100644", :type => "blob", :sha => "blob sha"}], {:base_tree => "sha2"}).once.returns(stub(:sha => "sha3"))
    octokit_client.expects(:update_ref).with("repo_name", "ref_name", "new commit sha")

    github_client = Undertaker::Mapper::GithubClient.new(octokit_client, 'repo_name', 'ref_name')
    github_client.push('', [FakeFile.new])
  end

It does test that .create_tree and .create_commit are called once with expected parameters.

However, because the implementation of GithubClient has so many nested invocations (e.g. .commit.tree.sha), I had to stub all those calls. It's very messy and worries me.

Also, even though I'm only calling push in my test, my stubbing assumes knowledge of calls made in private methods and I feel like I am testing implementation detail.

Am I doing this right? Also, what needs to be tested in a class like my GithubClient?

4
  • Also, I am testing the call to https://api.github.com/repos/... using WebMock in another test case. Do I need to test it when I am not directly making a call to the API? I think it's the responsibility of the writer of Octokit to unit test those calls, but I just wanted to make sure.
    – Eric
    Jan 7, 2020 at 18:34
  • The fact that you're making those calls in private methods is an implementation detail, the fact that you're making them at all is not. They're still interactions with a collaborator.
    – jonrsharpe
    Jan 7, 2020 at 19:07
  • @jonrsharpe But the test knows about the interactions in the private methods. Doesn't that go against the black-box testing principle?
    – Eric
    Jan 13, 2020 at 19:09
  • It doesn't know that they are in private methods, because it is a black box. If you refactored those interactions into the public methods, you wouldn't have to change the tests. The only issue here is that you're mocking something that you don't own.
    – jonrsharpe
    Jan 13, 2020 at 19:11

0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Browse other questions tagged or ask your own question.