Commit 655418bed2f81651c54988963713769f85d8b5da
1 parent
f5908cef
Exists in
master
and in
4 other branches
System hooks: fix broken tests
Showing
8 changed files
with
39 additions
and
38 deletions
Show diff stats
app/roles/git_push.rb
@@ -27,7 +27,7 @@ module GitPush | @@ -27,7 +27,7 @@ module GitPush | ||
27 | true | 27 | true |
28 | end | 28 | end |
29 | 29 | ||
30 | - def execute_web_hooks(oldrev, newrev, ref, user) | 30 | + def execute_hooks(oldrev, newrev, ref, user) |
31 | ref_parts = ref.split('/') | 31 | ref_parts = ref.split('/') |
32 | 32 | ||
33 | # Return if this is not a push to a branch (e.g. new commits) | 33 | # Return if this is not a push to a branch (e.g. new commits) |
@@ -35,7 +35,7 @@ module GitPush | @@ -35,7 +35,7 @@ module GitPush | ||
35 | 35 | ||
36 | data = post_receive_data(oldrev, newrev, ref, user) | 36 | data = post_receive_data(oldrev, newrev, ref, user) |
37 | 37 | ||
38 | - hooks.each { |web_hook| web_hook.execute(data) } | 38 | + hooks.each { |hook| hook.execute(data) } |
39 | end | 39 | end |
40 | 40 | ||
41 | def post_receive_data(oldrev, newrev, ref, user) | 41 | def post_receive_data(oldrev, newrev, ref, user) |
@@ -97,7 +97,7 @@ module GitPush | @@ -97,7 +97,7 @@ module GitPush | ||
97 | self.update_merge_requests(oldrev, newrev, ref, user) | 97 | self.update_merge_requests(oldrev, newrev, ref, user) |
98 | 98 | ||
99 | # Execute web hooks | 99 | # Execute web hooks |
100 | - self.execute_web_hooks(oldrev, newrev, ref, user) | 100 | + self.execute_hooks(oldrev, newrev, ref, user) |
101 | 101 | ||
102 | # Create satellite | 102 | # Create satellite |
103 | self.satellite.create unless self.satellite.exists? | 103 | self.satellite.create unless self.satellite.exists? |
spec/factories.rb
@@ -60,7 +60,7 @@ Factory.add(:key, Key) do |obj| | @@ -60,7 +60,7 @@ Factory.add(:key, Key) do |obj| | ||
60 | obj.key = File.read(File.join(Rails.root, "db", "pkey.example")) | 60 | obj.key = File.read(File.join(Rails.root, "db", "pkey.example")) |
61 | end | 61 | end |
62 | 62 | ||
63 | -Factory.add(:web_hook, WebHook) do |obj| | 63 | +Factory.add(:project_hook, ProjectHook) do |obj| |
64 | obj.url = Faker::Internet.uri("http") | 64 | obj.url = Faker::Internet.uri("http") |
65 | end | 65 | end |
66 | 66 |
spec/models/project_hooks_spec.rb
@@ -21,44 +21,44 @@ describe Project, "Hooks" do | @@ -21,44 +21,44 @@ describe Project, "Hooks" do | ||
21 | end | 21 | end |
22 | end | 22 | end |
23 | 23 | ||
24 | - describe "Web hooks" do | 24 | + describe "Project hooks" do |
25 | context "with no web hooks" do | 25 | context "with no web hooks" do |
26 | it "raises no errors" do | 26 | it "raises no errors" do |
27 | lambda { | 27 | lambda { |
28 | - project.execute_web_hooks('oldrev', 'newrev', 'ref', @user) | 28 | + project.execute_hooks('oldrev', 'newrev', 'ref', @user) |
29 | }.should_not raise_error | 29 | }.should_not raise_error |
30 | end | 30 | end |
31 | end | 31 | end |
32 | 32 | ||
33 | context "with web hooks" do | 33 | context "with web hooks" do |
34 | before do | 34 | before do |
35 | - @webhook = Factory(:web_hook) | ||
36 | - @webhook_2 = Factory(:web_hook) | ||
37 | - project.web_hooks << [@webhook, @webhook_2] | 35 | + @project_hook = Factory(:project_hook) |
36 | + @project_hook_2 = Factory(:project_hook) | ||
37 | + project.hooks << [@project_hook, @project_hook_2] | ||
38 | end | 38 | end |
39 | 39 | ||
40 | it "executes multiple web hook" do | 40 | it "executes multiple web hook" do |
41 | - @webhook.should_receive(:execute).once | ||
42 | - @webhook_2.should_receive(:execute).once | 41 | + @project_hook.should_receive(:execute).once |
42 | + @project_hook_2.should_receive(:execute).once | ||
43 | 43 | ||
44 | - project.execute_web_hooks('oldrev', 'newrev', 'refs/heads/master', @user) | 44 | + project.execute_hooks('oldrev', 'newrev', 'refs/heads/master', @user) |
45 | end | 45 | end |
46 | end | 46 | end |
47 | 47 | ||
48 | context "does not execute web hooks" do | 48 | context "does not execute web hooks" do |
49 | before do | 49 | before do |
50 | - @webhook = Factory(:web_hook) | ||
51 | - project.web_hooks << [@webhook] | 50 | + @project_hook = Factory(:project_hook) |
51 | + project.hooks << [@project_hook] | ||
52 | end | 52 | end |
53 | 53 | ||
54 | it "when pushing a branch for the first time" do | 54 | it "when pushing a branch for the first time" do |
55 | - @webhook.should_not_receive(:execute) | ||
56 | - project.execute_web_hooks('00000000000000000000000000000000', 'newrev', 'refs/heads/master', @user) | 55 | + @project_hook.should_not_receive(:execute) |
56 | + project.execute_hooks('00000000000000000000000000000000', 'newrev', 'refs/heads/master', @user) | ||
57 | end | 57 | end |
58 | 58 | ||
59 | it "when pushing tags" do | 59 | it "when pushing tags" do |
60 | - @webhook.should_not_receive(:execute) | ||
61 | - project.execute_web_hooks('oldrev', 'newrev', 'refs/tags/v1.0.0', @user) | 60 | + @project_hook.should_not_receive(:execute) |
61 | + project.execute_hooks('oldrev', 'newrev', 'refs/tags/v1.0.0', @user) | ||
62 | end | 62 | end |
63 | end | 63 | end |
64 | 64 |
spec/models/project_spec.rb
@@ -11,7 +11,7 @@ describe Project do | @@ -11,7 +11,7 @@ describe Project do | ||
11 | it { should have_many(:issues).dependent(:destroy) } | 11 | it { should have_many(:issues).dependent(:destroy) } |
12 | it { should have_many(:notes).dependent(:destroy) } | 12 | it { should have_many(:notes).dependent(:destroy) } |
13 | it { should have_many(:snippets).dependent(:destroy) } | 13 | it { should have_many(:snippets).dependent(:destroy) } |
14 | - it { should have_many(:web_hooks).dependent(:destroy) } | 14 | + it { should have_many(:hooks).dependent(:destroy) } |
15 | it { should have_many(:deploy_keys).dependent(:destroy) } | 15 | it { should have_many(:deploy_keys).dependent(:destroy) } |
16 | end | 16 | end |
17 | 17 |
spec/models/web_hook_spec.rb
1 | require 'spec_helper' | 1 | require 'spec_helper' |
2 | 2 | ||
3 | -describe WebHook do | 3 | +describe ProjectHook do |
4 | describe "Associations" do | 4 | describe "Associations" do |
5 | it { should belong_to :project } | 5 | it { should belong_to :project } |
6 | end | 6 | end |
@@ -23,32 +23,32 @@ describe WebHook do | @@ -23,32 +23,32 @@ describe WebHook do | ||
23 | 23 | ||
24 | describe "execute" do | 24 | describe "execute" do |
25 | before(:each) do | 25 | before(:each) do |
26 | - @webhook = Factory :web_hook | 26 | + @project_hook = Factory :project_hook |
27 | @project = Factory :project | 27 | @project = Factory :project |
28 | - @project.web_hooks << [@webhook] | 28 | + @project.hooks << [@project_hook] |
29 | @data = { before: 'oldrev', after: 'newrev', ref: 'ref'} | 29 | @data = { before: 'oldrev', after: 'newrev', ref: 'ref'} |
30 | 30 | ||
31 | - WebMock.stub_request(:post, @webhook.url) | 31 | + WebMock.stub_request(:post, @project_hook.url) |
32 | end | 32 | end |
33 | 33 | ||
34 | it "POSTs to the web hook URL" do | 34 | it "POSTs to the web hook URL" do |
35 | - @webhook.execute(@data) | ||
36 | - WebMock.should have_requested(:post, @webhook.url).once | 35 | + @project_hook.execute(@data) |
36 | + WebMock.should have_requested(:post, @project_hook.url).once | ||
37 | end | 37 | end |
38 | 38 | ||
39 | it "POSTs the data as JSON" do | 39 | it "POSTs the data as JSON" do |
40 | json = @data.to_json | 40 | json = @data.to_json |
41 | 41 | ||
42 | - @webhook.execute(@data) | ||
43 | - WebMock.should have_requested(:post, @webhook.url).with(body: json).once | 42 | + @project_hook.execute(@data) |
43 | + WebMock.should have_requested(:post, @project_hook.url).with(body: json).once | ||
44 | end | 44 | end |
45 | 45 | ||
46 | it "catches exceptions" do | 46 | it "catches exceptions" do |
47 | WebHook.should_receive(:post).and_raise("Some HTTP Post error") | 47 | WebHook.should_receive(:post).and_raise("Some HTTP Post error") |
48 | 48 | ||
49 | lambda { | 49 | lambda { |
50 | - @webhook.execute(@data) | ||
51 | - }.should_not raise_error | 50 | + @project_hook.execute(@data) |
51 | + }.should raise_error | ||
52 | end | 52 | end |
53 | end | 53 | end |
54 | end | 54 | end |
spec/requests/admin/security_spec.rb
@@ -13,9 +13,9 @@ describe "Admin::Projects" do | @@ -13,9 +13,9 @@ describe "Admin::Projects" do | ||
13 | it { admin_users_path.should be_denied_for :visitor } | 13 | it { admin_users_path.should be_denied_for :visitor } |
14 | end | 14 | end |
15 | 15 | ||
16 | - describe "GET /admin/emails" do | ||
17 | - it { admin_emails_path.should be_allowed_for :admin } | ||
18 | - it { admin_emails_path.should be_denied_for :user } | ||
19 | - it { admin_emails_path.should be_denied_for :visitor } | 16 | + describe "GET /admin/hooks" do |
17 | + it { admin_hooks_path.should be_allowed_for :admin } | ||
18 | + it { admin_hooks_path.should be_denied_for :user } | ||
19 | + it { admin_hooks_path.should be_denied_for :visitor } | ||
20 | end | 20 | end |
21 | end | 21 | end |
spec/requests/hooks_spec.rb
@@ -9,7 +9,7 @@ describe "Hooks" do | @@ -9,7 +9,7 @@ describe "Hooks" do | ||
9 | 9 | ||
10 | describe "GET index" do | 10 | describe "GET index" do |
11 | it "should be available" do | 11 | it "should be available" do |
12 | - @hook = Factory :web_hook, :project => @project | 12 | + @hook = Factory :project_hook, :project => @project |
13 | visit project_hooks_path(@project) | 13 | visit project_hooks_path(@project) |
14 | page.should have_content "Hooks" | 14 | page.should have_content "Hooks" |
15 | page.should have_content @hook.url | 15 | page.should have_content @hook.url |
@@ -21,7 +21,7 @@ describe "Hooks" do | @@ -21,7 +21,7 @@ describe "Hooks" do | ||
21 | @url = Faker::Internet.uri("http") | 21 | @url = Faker::Internet.uri("http") |
22 | visit project_hooks_path(@project) | 22 | visit project_hooks_path(@project) |
23 | fill_in "hook_url", :with => @url | 23 | fill_in "hook_url", :with => @url |
24 | - expect { click_button "Add Web Hook" }.to change(WebHook, :count).by(1) | 24 | + expect { click_button "Add Web Hook" }.to change(ProjectHook, :count).by(1) |
25 | end | 25 | end |
26 | 26 | ||
27 | it "should open new team member popup" do | 27 | it "should open new team member popup" do |
@@ -32,7 +32,8 @@ describe "Hooks" do | @@ -32,7 +32,8 @@ describe "Hooks" do | ||
32 | 32 | ||
33 | describe "Test" do | 33 | describe "Test" do |
34 | before do | 34 | before do |
35 | - @hook = Factory :web_hook, :project => @project | 35 | + @hook = Factory :project_hook, :project => @project |
36 | + stub_request(:post, @hook.url) | ||
36 | visit project_hooks_path(@project) | 37 | visit project_hooks_path(@project) |
37 | click_link "Test Hook" | 38 | click_link "Test Hook" |
38 | end | 39 | end |
spec/workers/post_receive_spec.rb
@@ -22,14 +22,14 @@ describe PostReceive do | @@ -22,14 +22,14 @@ describe PostReceive do | ||
22 | Key.stub(find_by_identifier: nil) | 22 | Key.stub(find_by_identifier: nil) |
23 | 23 | ||
24 | project.should_not_receive(:observe_push) | 24 | project.should_not_receive(:observe_push) |
25 | - project.should_not_receive(:execute_web_hooks) | 25 | + project.should_not_receive(:execute_hooks) |
26 | 26 | ||
27 | PostReceive.perform(project.path, 'sha-old', 'sha-new', 'refs/heads/master', key_id).should be_false | 27 | PostReceive.perform(project.path, 'sha-old', 'sha-new', 'refs/heads/master', key_id).should be_false |
28 | end | 28 | end |
29 | 29 | ||
30 | it "asks the project to execute web hooks" do | 30 | it "asks the project to execute web hooks" do |
31 | Project.stub(find_by_path: project) | 31 | Project.stub(find_by_path: project) |
32 | - project.should_receive(:execute_web_hooks).with('sha-old', 'sha-new', 'refs/heads/master', project.owner) | 32 | + project.should_receive(:execute_hooks).with('sha-old', 'sha-new', 'refs/heads/master', project.owner) |
33 | 33 | ||
34 | PostReceive.perform(project.path, 'sha-old', 'sha-new', 'refs/heads/master', key_id) | 34 | PostReceive.perform(project.path, 'sha-old', 'sha-new', 'refs/heads/master', key_id) |
35 | end | 35 | end |