From d099494ad4432f20442a8cf69a8d58f9af99b46c Mon Sep 17 00:00:00 2001 From: Jakob Skjerning Date: Sun, 18 Oct 2015 13:01:20 +0200 Subject: [PATCH] Clean up syntax --- app/services/github_hook/updater.rb | 31 +++++++++------ .../functional/github_hook_controller_test.rb | 5 ++- test/unit/github_hook/updater_test.rb | 39 +++++++++++++++---- 3 files changed, 56 insertions(+), 19 deletions(-) diff --git a/app/services/github_hook/updater.rb b/app/services/github_hook/updater.rb index 63d07d3..7e3044d 100644 --- a/app/services/github_hook/updater.rb +++ b/app/services/github_hook/updater.rb @@ -64,7 +64,8 @@ def exec(command, directory) logfile.unlink if logfile && logfile.respond_to?(:unlink) end - # Finds the Redmine project in the database based on the given project identifier + # Finds the Redmine project in the database based on the given project + # identifier def find_project identifier = get_identifier project = Project.find_by_identifier(identifier.downcase) @@ -80,12 +81,15 @@ def find_repositories end if repositories.nil? || repositories.length == 0 - fail TypeError, "Project '#{project}' ('#{project.identifier}') has no repository" + fail( + TypeError, + "Project '#{project}' ('#{project.identifier}') has no repository" + ) end - # if a specific repository id is passed in url parameter "repository_id", then try to find it in - # the list of current project repositories and use only this and not all to pull changes from - # (issue #54) + # if a specific repository id is passed in url parameter "repository_id", + # then try to find it in the list of current project repositories and use + # only this and not all to pull changes from (issue #54) if params.key?(:repository_id) param_repo = repositories.select do |repo| repo.identifier == params[:repository_id] @@ -102,18 +106,21 @@ def find_repositories repositories end - # Gets the project identifier from the querystring parameters and if that's not supplied, assume - # the Github repository name is the same as the project identifier. + # Gets the project identifier from the querystring parameters and if that's + # not supplied, assume the Github repository name is the same as the project + # identifier. def get_identifier identifier = get_project_name fail ActiveRecord::RecordNotFound, "Project identifier not specified" if identifier.nil? identifier end - # Attempts to find the project name. It first looks in the params, then in the - # payload if params[:project_id] isn't given. + # Attempts to find the project name. It first looks in the params, then in + # the payload if params[:project_id] isn't given. def get_project_name - params[:project_id] || (payload["repository"] ? payload["repository"]["name"] : nil) + project_id = params[:project_id] + name_from_repository = payload.fetch("repository", {}).fetch("name", nil) + project_id || name_from_repository end def git_command(command) @@ -136,7 +143,9 @@ def time_diff_milli(start, finish) def update_repository(repository) command = git_command("fetch origin") if exec(command, repository.url) - command = git_command("fetch --prune origin \"+refs/heads/*:refs/heads/*\"") + command = git_command( + "fetch --prune origin \"+refs/heads/*:refs/heads/*\"" + ) exec(command, repository.url) end end diff --git a/test/functional/github_hook_controller_test.rb b/test/functional/github_hook_controller_test.rb index 39ce4fb..fa012a5 100644 --- a/test/functional/github_hook_controller_test.rb +++ b/test/functional/github_hook_controller_test.rb @@ -84,7 +84,10 @@ def test_should_render_response_from_github_hook_when_done end def test_should_render_error_message - GithubHook::Updater.any_instance.expects(:update_repository).raises(ActiveRecord::RecordNotFound.new("Repository not found")) + GithubHook::Updater + .any_instance + .expects(:update_repository) + .raises(ActiveRecord::RecordNotFound.new("Repository not found")) do_post assert_response :not_found assert_equal({ diff --git a/test/unit/github_hook/updater_test.rb b/test/unit/github_hook/updater_test.rb index e4a83fa..babab37 100644 --- a/test/unit/github_hook/updater_test.rb +++ b/test/unit/github_hook/updater_test.rb @@ -20,10 +20,21 @@ def repository @repository end + # rubocop:disable Metrics/LineLength def payload # Ruby hash with the parsed data from the JSON payload - {"before" => "5aef35982fb2d34e9d9d4502f6ede1072793222d", "repository" => {"url" => "http://github.com/defunkt/github", "name" => "github", "description" => "You're lookin' at it.", "watchers" => 5, "forks" => 2, "private" => 1, "owner" => {"email" => "chris@ozmm.org", "name" => "defunkt"}}, "commits" => [{"id" => "41a212ee83ca127e3c8cf465891ab7216a705f59", "url" => "http://github.com/defunkt/github/commit/41a212ee83ca127e3c8cf465891ab7216a705f59", "author" => {"email" => "chris@ozmm.org", "name" => "Chris Wanstrath"}, "message" => "okay i give in", "timestamp" => "2008-02-15T14:57:17-08:00", "added" => ["filepath.rb"]}, {"id" => "de8251ff97ee194a289832576287d6f8ad74e3d0", "url" => "http://github.com/defunkt/github/commit/de8251ff97ee194a289832576287d6f8ad74e3d0", "author" => {"email" => "chris@ozmm.org", "name" => "Chris Wanstrath"}, "message" => "update pricing a tad", "timestamp" => "2008-02-15T14:36:34-08:00"}], "after" => "de8251ff97ee194a289832576287d6f8ad74e3d0", "ref" => "refs/heads/master"} - end + { + "before" => "5aef35982fb2d34e9d9d4502f6ede1072793222d", + "repository" => {"url" => "http://github.com/defunkt/github", "name" => "github", "description" => "You're lookin' at it.", "watchers" => 5, "forks" => 2, "private" => 1, "owner" => {"email" => "chris@ozmm.org", "name" => "defunkt"}}, + "commits" => [ + {"id" => "41a212ee83ca127e3c8cf465891ab7216a705f59", "url" => "http://github.com/defunkt/github/commit/41a212ee83ca127e3c8cf465891ab7216a705f59", "author" => {"email" => "chris@ozmm.org", "name" => "Chris Wanstrath"}, "message" => "okay i give in", "timestamp" => "2008-02-15T14:57:17-08:00", "added" => ["filepath.rb"]}, + {"id" => "de8251ff97ee194a289832576287d6f8ad74e3d0", "url" => "http://github.com/defunkt/github/commit/de8251ff97ee194a289832576287d6f8ad74e3d0", "author" => {"email" => "chris@ozmm.org", "name" => "Chris Wanstrath"}, "message" => "update pricing a tad", "timestamp" => "2008-02-15T14:36:34-08:00"} + ], + "after" => "de8251ff97ee194a289832576287d6f8ad74e3d0", + "ref" => "refs/heads/master" + } + end + # rubocop:enable Metrics/LineLength def build_updater(payload, options = {}) updater = GithubHook::Updater.new(payload, options) @@ -59,14 +70,28 @@ def test_fetches_changes_from_origin end def test_resets_repository_when_fetch_origin_succeeds - updater.expects(:exec).with("git fetch origin", repository.url).returns(true) - updater.expects(:exec).with("git fetch --prune origin \"+refs/heads/*:refs/heads/*\"", repository.url) + updater + .expects(:exec) + .with("git fetch origin", repository.url) + .returns(true) + updater + .expects(:exec) + .with( + "git fetch --prune origin \"+refs/heads/*:refs/heads/*\"", + repository.url + ) updater.call end def test_resets_repository_when_fetch_origin_fails - updater.expects(:exec).with("git fetch origin", repository.url).returns(false) - updater.expects(:exec).with("git reset --soft refs\/remotes\/origin\/master", repository.url).never + updater + .expects(:exec) + .with("git fetch origin", repository.url) + .returns(false) + updater + .expects(:exec) + .with("git reset --soft refs\/remotes\/origin\/master", repository.url) + .never updater.call end @@ -91,7 +116,7 @@ def test_updates_only_the_specified_repository another_repository.expects(:fetch_changesets).never project.repositories << another_repository - updater = build_updater(payload, {:repository_id => "redmine"}) + updater = build_updater(payload, :repository_id => "redmine") updater.expects(:exec).with("git fetch origin", repository.url) updater.call end