Fix a bug where the `disable` action did not work in SchedulerAgent.

Bad specs are fixed and specs that should be common among agents that
include AgentControllerConcern have been extracted to
spec/support/shared_examples/agent_controller_concern.rb.

Akinori MUSHA %!s(int64=10) %!d(string=hace) años
padre
commit
c1a2325d40

+ 26 - 7
app/concerns/agent_controller_concern.rb

@@ -30,18 +30,37 @@ module AgentControllerConcern
30 30
   end
31 31
 
32 32
   def control!
33
-    control_targets.active.each { |target|
33
+    control_targets.each { |target|
34 34
       begin
35 35
         case control_action
36 36
         when 'run'
37
-          log "Agent run queued for '#{target.name}'"
38
-          Agent.async_check(target.id)
37
+          case
38
+          when target.cannot_be_scheduled?
39
+            error "'#{target.name}' cannot run without an incoming event"
40
+          when target.disabled?
41
+            log "Agent run ignored for disabled Agent '#{target.name}'"
42
+          else
43
+            Agent.async_check(target.id)
44
+            log "Agent run queued for '#{target.name}'"
45
+          end
39 46
         when 'enable'
40
-          log "Enabling the Agent '#{target.name}'"
41
-          target.update!(disable: false) if target.disabled?
47
+          case
48
+          when target.disabled?
49
+            target.update!(disabled: false)
50
+            log "Agent '#{target.name}' is enabled"
51
+          else
52
+            log "Agent '#{target.name}' is already enabled"
53
+          end
42 54
         when 'disable'
43
-          log "Disabling the Agent '#{target.name}'"
44
-          target.update!(disable: true) unless target.disabled?
55
+          case
56
+          when target.disabled?
57
+            log "Agent '#{target.name}' is alread disabled"
58
+          else
59
+            target.update!(disabled: true)
60
+            log "Agent '#{target.name}' is disabled"
61
+          end
62
+        else
63
+          error "Unsupported action '#{control_action}' ignored for '#{target.name}'"
45 64
         end
46 65
       rescue => e
47 66
         error "Failed to #{control_action} '#{target.name}': #{e.message}"

+ 53 - 105
spec/models/agents/scheduler_agent_spec.rb

@@ -1,96 +1,65 @@
1 1
 require 'spec_helper'
2 2
 
3 3
 describe Agents::SchedulerAgent do
4
-  before do
5
-    @agent = Agents::SchedulerAgent.new(name: 'Example', options: { 'schedule' => '0 * * * *' })
6
-    @agent.user = users(:bob)
7
-    @agent.save
8
-  end
4
+  let(:valid_params) {
5
+    {
6
+      name: 'Example',
7
+      options: { 'schedule' => '0 * * * *' },
8
+    }
9
+  }
9 10
 
10
-  describe "validation" do
11
-    it "should validate action" do
12
-      ['run', 'enable', 'disable', '', nil].each { |action|
13
-        @agent.options['action'] = action
14
-        expect(@agent).to be_valid
15
-      }
16
-
17
-      ['delete', 1, true].each { |action|
18
-        @agent.options['action'] = action
19
-        expect(@agent).not_to be_valid
20
-      }
21
-    end
22
-
23
-    it "should validate schedule" do
24
-      expect(@agent).to be_valid
11
+  let(:agent) {
12
+    described_class.create!(valid_params) { |agent|
13
+      agent.user = users(:bob)
14
+    }
15
+  }
25 16
 
26
-      @agent.options.delete('schedule')
27
-      expect(@agent).not_to be_valid
17
+  it_behaves_like AgentControllerConcern
28 18
 
29
-      @agent.options['schedule'] = nil
30
-      expect(@agent).not_to be_valid
31
-
32
-      @agent.options['schedule'] = ''
33
-      expect(@agent).not_to be_valid
19
+  describe "validation" do
20
+    it "should validate schedule" do
21
+      expect(agent).to be_valid
34 22
 
35
-      @agent.options['schedule'] = '0'
36
-      expect(@agent).not_to be_valid
23
+      agent.options.delete('schedule')
24
+      expect(agent).not_to be_valid
37 25
 
38
-      @agent.options['schedule'] = '*/15 * * * * * *'
39
-      expect(@agent).not_to be_valid
26
+      agent.options['schedule'] = nil
27
+      expect(agent).not_to be_valid
40 28
 
41
-      @agent.options['schedule'] = '*/1 * * * *'
42
-      expect(@agent).to be_valid
29
+      agent.options['schedule'] = ''
30
+      expect(agent).not_to be_valid
43 31
 
44
-      @agent.options['schedule'] = '*/1 * * *'
45
-      expect(@agent).not_to be_valid
32
+      agent.options['schedule'] = '0'
33
+      expect(agent).not_to be_valid
46 34
 
47
-      stub(@agent).second_precision_enabled { true }
48
-      @agent.options['schedule'] = '*/15 * * * * *'
49
-      expect(@agent).to be_valid
35
+      agent.options['schedule'] = '*/15 * * * * * *'
36
+      expect(agent).not_to be_valid
50 37
 
51
-      stub(@agent).second_precision_enabled { false }
52
-      @agent.options['schedule'] = '*/10 * * * * *'
53
-      expect(@agent).not_to be_valid
38
+      agent.options['schedule'] = '*/1 * * * *'
39
+      expect(agent).to be_valid
54 40
 
55
-      @agent.options['schedule'] = '5/30 * * * * *'
56
-      expect(@agent).not_to be_valid
41
+      agent.options['schedule'] = '*/1 * * *'
42
+      expect(agent).not_to be_valid
57 43
 
58
-      @agent.options['schedule'] = '*/15 * * * * *'
59
-      expect(@agent).to be_valid
44
+      stub(agent).second_precision_enabled { true }
45
+      agent.options['schedule'] = '*/15 * * * * *'
46
+      expect(agent).to be_valid
60 47
 
61
-      @agent.options['schedule'] = '15,45 * * * * *'
62
-      expect(@agent).to be_valid
48
+      stub(agent).second_precision_enabled { false }
49
+      agent.options['schedule'] = '*/10 * * * * *'
50
+      expect(agent).not_to be_valid
63 51
 
64
-      @agent.options['schedule'] = '0 * * * * *'
65
-      expect(@agent).to be_valid
66
-    end
67
-  end
52
+      agent.options['schedule'] = '5/30 * * * * *'
53
+      expect(agent).not_to be_valid
68 54
 
69
-  describe 'control_action' do
70
-    it "should be one of the supported values" do
71
-      ['run', '', nil].each { |action|
72
-        @agent.options['action'] = action
73
-        expect(@agent.control_action).to eq('run')
74
-      }
75
-
76
-      ['enable', 'disable'].each { |action|
77
-        @agent.options['action'] = action
78
-        expect(@agent.control_action).to eq(action)
79
-      }
80
-    end
55
+      agent.options['schedule'] = '*/15 * * * * *'
56
+      expect(agent).to be_valid
81 57
 
82
-    it "cannot be 'run' if any of the control targets cannot be scheduled" do
83
-      expect(@agent.control_action).to eq('run')
84
-      @agent.control_targets = [agents(:bob_rain_notifier_agent)]
85
-      expect(@agent).not_to be_valid
86
-    end
58
+      agent.options['schedule'] = '15,45 * * * * *'
59
+      expect(agent).to be_valid
87 60
 
88
-    it "can be 'enable' or 'disable' no matter if control targets can be scheduled or not" do
89
-      ['enable', 'disable'].each { |action|
90
-        @agent.options['action'] = action
91
-        @agent.control_targets = [agents(:bob_rain_notifier_agent)]
92
-        expect(@agent).to be_valid
93
-      }
61
+      agent.options['schedule'] = '0 * * * * *'
62
+      expect(agent).to be_valid
94 63
     end
95 64
   end
96 65
 
@@ -98,43 +67,22 @@ describe Agents::SchedulerAgent do
98 67
     it "should delete memory['scheduled_at'] if and only if options is changed" do
99 68
       time = Time.now.to_i
100 69
 
101
-      @agent.memory['scheduled_at'] = time
102
-      @agent.save
103
-      expect(@agent.memory['scheduled_at']).to eq(time)
70
+      agent.memory['scheduled_at'] = time
71
+      agent.save
72
+      expect(agent.memory['scheduled_at']).to eq(time)
104 73
 
105
-      @agent.memory['scheduled_at'] = time
106
-      # Currently @agent.options[]= is not detected
107
-      @agent.options = { 'schedule' => '*/5 * * * *' }
108
-      @agent.save
109
-      expect(@agent.memory['scheduled_at']).to be_nil
74
+      agent.memory['scheduled_at'] = time
75
+      # Currently agent.options[]= is not detected
76
+      agent.options = { 'schedule' => '*/5 * * * *' }
77
+      agent.save
78
+      expect(agent.memory['scheduled_at']).to be_nil
110 79
     end
111 80
   end
112 81
 
113 82
   describe "check!" do
114 83
     it "should control targets" do
115
-      control_targets = [agents(:bob_website_agent), agents(:bob_weather_agent)]
116
-      @agent.control_targets = control_targets
117
-      @agent.save!
118
-
119
-      control_target_ids = control_targets.map(&:id)
120
-      stub(Agent).async_check(anything) { |id|
121
-        control_target_ids.delete(id)
122
-      }
123
-
124
-      @agent.check!
125
-      expect(control_target_ids).to be_empty
126
-
127
-      @agent.options['action'] = 'disable'
128
-      @agent.save!
129
-
130
-      @agent.check!
131
-      control_targets.all? { |control_target| control_target.disabled? }
132
-
133
-      @agent.options['action'] = 'enable'
134
-      @agent.save!
135
-
136
-      @agent.check!
137
-      control_targets.all? { |control_target| !control_target.disabled? }
84
+      stub(agent).control!.once { nil }
85
+      agent.check!
138 86
     end
139 87
   end
140 88
 end

+ 99 - 0
spec/support/shared_examples/agent_controller_concern.rb

@@ -0,0 +1,99 @@
1
+require 'spec_helper'
2
+
3
+shared_examples_for AgentControllerConcern do
4
+  describe "preconditions" do
5
+    it "must be satisfied for these shared examples" do
6
+      expect(agent.user).to eq(users(:bob))
7
+      expect(agent.control_action).to eq('run')
8
+    end
9
+  end
10
+
11
+  describe "validation" do
12
+    it "should validate action" do
13
+      ['run', 'enable', 'disable', '', nil].each { |action|
14
+        agent.options['action'] = action
15
+        expect(agent).to be_valid
16
+      }
17
+
18
+      ['delete', 1, true].each { |action|
19
+        agent.options['action'] = action
20
+        expect(agent).not_to be_valid
21
+      }
22
+    end
23
+  end
24
+
25
+  describe 'control_action' do
26
+    it "should be one of the supported values" do
27
+      ['run', '', nil].each { |action|
28
+        agent.options['action'] = action
29
+        expect(agent.control_action).to eq('run')
30
+      }
31
+
32
+      ['enable', 'disable'].each { |action|
33
+        agent.options['action'] = action
34
+        expect(agent.control_action).to eq(action)
35
+      }
36
+    end
37
+
38
+    it "cannot be 'run' if any of the control targets cannot be scheduled" do
39
+      expect(agent.control_action).to eq('run')
40
+      agent.control_targets = [agents(:bob_rain_notifier_agent)]
41
+      expect(agent).not_to be_valid
42
+    end
43
+
44
+    it "can be 'enable' or 'disable' no matter if control targets can be scheduled or not" do
45
+      ['enable', 'disable'].each { |action|
46
+        agent.options['action'] = action
47
+        agent.control_targets = [agents(:bob_rain_notifier_agent)]
48
+        expect(agent).to be_valid
49
+      }
50
+    end
51
+  end
52
+
53
+  describe "control!" do
54
+    before do
55
+      agent.control_targets = [agents(:bob_website_agent), agents(:bob_weather_agent)]
56
+      agent.save!
57
+    end
58
+
59
+    it "should run targets" do
60
+      control_target_ids = agent.control_targets.map(&:id)
61
+      stub(Agent).async_check(anything) { |id|
62
+        control_target_ids.delete(id)
63
+      }
64
+
65
+      agent.control!
66
+      expect(control_target_ids).to be_empty
67
+    end
68
+
69
+    it "should not run disabled targets" do
70
+      control_target_ids = agent.control_targets.map(&:id)
71
+      stub(Agent).async_check(anything) { |id|
72
+        control_target_ids.delete(id)
73
+      }
74
+
75
+      agent.control_targets.last.update!(disabled: true)
76
+
77
+      agent.control!
78
+      expect(control_target_ids).to eq [agent.control_targets.last.id]
79
+    end
80
+
81
+    it "should enable targets" do
82
+      agent.options['action'] = 'disable'
83
+      agent.save!
84
+      agent.control_targets.first.update!(disabled: true)
85
+
86
+      agent.control!
87
+      expect(agent.control_targets.reload).to all(be_disabled)
88
+    end
89
+
90
+    it "should disable targets" do
91
+      agent.options['action'] = 'enable'
92
+      agent.save!
93
+      agent.control_targets.first.update!(disabled: true)
94
+
95
+      agent.control!
96
+      expect(agent.control_targets.reload).to all(satisfy { |a| !a.disabled? })
97
+    end
98
+  end
99
+end