Skip to content

Commit bd79971

Browse files
committed
spec: drop compare_json
it had critical bugs where mismatched keys would bet let through. so instead just compare directly; rspec has good hash and array diffing nowadays, and we just need to call as_json on a few things Change-Id: I6147e8c988c1dc4f4e3224dc6a4e51e51b27e5f3 Reviewed-on: https://gerrit.instructure.com/104402 Tested-by: Jenkins Reviewed-by: Simon Williams <simon@instructure.com> Product-Review: Cody Cutrer <cody@instructure.com> QA-Review: Cody Cutrer <cody@instructure.com>
1 parent f105926 commit bd79971

6 files changed

Lines changed: 37 additions & 46 deletions

File tree

app/models/assignment.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,7 @@ def moderation_setting_ok?
228228
moderated_grading
229229
grades_published_at
230230
omit_from_final_grade
231+
grading_standard_id
231232
)
232233

233234
def external_tool?

spec/apis/v1/assignment_groups_api_spec.rb

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -162,27 +162,31 @@ def setup_grading_periods
162162

163163
expected = [
164164
{
165-
'group_weight' => 60,
165+
'group_weight' => 60.0,
166166
'id' => @group2.id,
167167
'name' => 'group2',
168168
'position' => 7,
169169
'rules' => {},
170170
'any_assignment_in_closed_grading_period' => false,
171+
'integration_data' => {},
172+
'sis_source_id' => nil,
171173
'assignments' => [
172-
controller.assignment_json(@a3,@user,session),
173-
controller.assignment_json(@a4,@user,session, include_discussion_topic: true)
174+
controller.assignment_json(@a3,@user,session).as_json,
175+
controller.assignment_json(@a4,@user,session, include_discussion_topic: false).as_json
174176
]
175177
},
176178
{
177-
'group_weight' => 40,
179+
'group_weight' => 40.0,
178180
'id' => @group1.id,
179181
'name' => 'group1',
180182
'position' => 10,
181183
'rules' => {},
182184
'any_assignment_in_closed_grading_period' => false,
185+
'integration_data' => {},
186+
'sis_source_id' => nil,
183187
'assignments' => [
184-
controller.assignment_json(@a1,@user,session),
185-
controller.assignment_json(@a2,@user,session)
188+
controller.assignment_json(@a1,@user,session).as_json,
189+
controller.assignment_json(@a2,@user,session).as_json
186190
]
187191
}
188192
]
@@ -193,7 +197,7 @@ def setup_grading_periods
193197
end
194198
end
195199

196-
compare_json(json, expected)
200+
expect(json).to eq expected
197201
end
198202

199203
context "exclude response fields" do
@@ -385,20 +389,22 @@ def setup_grading_periods
385389

386390
expected = [
387391
{
388-
'group_weight' => 40,
392+
'group_weight' => 40.0,
389393
'id' => group.id,
390394
'name' => 'group1',
391395
'position' => 10,
392396
'rules' => {},
393397
'any_assignment_in_closed_grading_period' => false,
398+
'integration_data' => {},
399+
'sis_source_id' => nil,
394400
'assignments' => [
395-
controller.assignment_json(a1, @user,session),
396-
controller.assignment_json(a2, @user,session)
401+
controller.assignment_json(a1, @user,session).as_json,
402+
controller.assignment_json(a2, @user,session).as_json
397403
]
398404
}
399405
]
400406

401-
compare_json(json, expected)
407+
expect(json).to eq expected
402408
end
403409

404410
it "should include all dates" do
@@ -426,20 +432,22 @@ def setup_grading_periods
426432

427433
expected = [
428434
{
429-
'group_weight' => 40,
435+
'group_weight' => 40.0,
430436
'id' => group.id,
431437
'name' => 'group1',
432438
'position' => 10,
433439
'rules' => {},
434440
'any_assignment_in_closed_grading_period' => false,
441+
'integration_data' => {},
442+
'sis_source_id' => nil,
435443
'assignments' => [
436-
controller.assignment_json(a1, @user,session, include_all_dates: true),
437-
controller.assignment_json(a2, @user,session, include_all_dates: true)
444+
controller.assignment_json(a1, @user,session, include_all_dates: true).as_json,
445+
controller.assignment_json(a2, @user,session, include_all_dates: true).as_json
438446
]
439447
}
440448
]
441449

442-
compare_json(json, expected)
450+
expect(json).to eq expected
443451
end
444452

445453
it "should exclude deleted assignments" do

spec/apis/v1/context_module_items_api_spec.rb

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,10 +135,11 @@
135135
"indent" => 1,
136136
"completion_requirement" => { "type" => "must_view" },
137137
"published" => true,
138-
"module_id" => @module1.id
138+
"module_id" => @module1.id,
139+
"new_tab" => nil
139140
}
140141
]
141-
compare_json(json, expected)
142+
expect(json).to eq expected
142143
end
143144

144145
context 'index with content details' do

spec/apis/v1/todo_items_api_spec.rb

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@
2323
include Api
2424
include Api::V1::Assignment
2525
def update_assignment_json
26-
@a1_json['assignment'] = controller.assignment_json(@a1,@user,session)
27-
@a2_json['assignment'] = controller.assignment_json(@a2,@user,session)
26+
@a1_json['assignment'] = controller.assignment_json(@a1,@user,session).as_json
27+
@a2_json['assignment'] = controller.assignment_json(@a2,@user,session).as_json
2828
end
2929

3030
def strip_secure_params(json)
@@ -100,8 +100,8 @@ def another_submission
100100
:controller => "users", :action => "todo_items", :format => "json")
101101
update_assignment_json
102102
json = json.sort_by { |t| t['assignment']['id'] }
103-
compare_json strip_secure_params(json.first), strip_secure_params(@a1_json)
104-
compare_json strip_secure_params(json.second), strip_secure_params(@a2_json)
103+
expect(strip_secure_params(json.first)).to eq strip_secure_params(@a1_json)
104+
expect(strip_secure_params(json.second)).to eq strip_secure_params(@a2_json)
105105
end
106106

107107
it "returns a course-specific todo list for a student" do
@@ -111,7 +111,7 @@ def another_submission
111111
.first
112112

113113
update_assignment_json
114-
compare_json(strip_secure_params(json), strip_secure_params(@a1_json))
114+
expect(strip_secure_params(json)).to eq strip_secure_params(@a1_json)
115115
end
116116

117117
it "returns a course-specific todo list for a teacher" do
@@ -120,7 +120,7 @@ def another_submission
120120
:format => "json", :course_id => @teacher_course.to_param)
121121
.first
122122
update_assignment_json
123-
compare_json(strip_secure_params(json), strip_secure_params(@a2_json))
123+
expect(strip_secure_params(json)).to eq strip_secure_params(@a2_json)
124124
end
125125

126126
it "should return a list for users who are both teachers and students" do
@@ -131,8 +131,8 @@ def another_submission
131131
@a1_json.deep_merge!({ 'assignment' => { 'needs_grading_count' => 0 } })
132132
json = json.sort_by { |t| t['assignment']['id'] }
133133
update_assignment_json
134-
compare_json(strip_secure_params(json.first), strip_secure_params(@a1_json))
135-
compare_json(strip_secure_params(json.second), strip_secure_params(@a2_json))
134+
expect(strip_secure_params(json.first)).to eq strip_secure_params(@a1_json)
135+
expect(strip_secure_params(json.second)).to eq strip_secure_params(@a2_json)
136136
end
137137

138138
it "should ignore a todo item permanently" do
@@ -170,7 +170,7 @@ def another_submission
170170
@a2_json['needs_grading_count'] = 2
171171
@a2_json['assignment']['needs_grading_count'] = 2
172172
update_assignment_json
173-
compare_json(strip_secure_params(json.first), strip_secure_params(@a2_json))
173+
expect(strip_secure_params(json.first)).to eq strip_secure_params(@a2_json)
174174
end
175175

176176
it "should ignore excused assignments for students" do

spec/lib/services/notification_service_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ def send_message(message_body: , queue_url: )
129129
NotificationService.stubs(:notification_queue).returns(spy)
130130

131131
NotificationService.process(1, 'hello', 'email', 'alice@example.com')
132-
compare_json(expected, spy.sent_hash)
132+
expect(expected).to eq spy.sent_hash
133133
end
134134
end
135135
end

spec/spec_helper.rb

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -765,25 +765,6 @@ def content_type_key
765765
'Content-Type'
766766
end
767767

768-
def compare_json(actual, expected)
769-
if actual.is_a?(Hash)
770-
actual.each do |k, v|
771-
expected_v = expected[k]
772-
compare_json(v, expected_v)
773-
end
774-
elsif actual.is_a?(Array)
775-
actual.zip(expected).each do |a, e|
776-
compare_json(a, e)
777-
end
778-
else
779-
if actual.is_a?(Integer) || actual.is_a?(Float)
780-
expect(actual).to eq expected
781-
else
782-
expect(actual.to_json).to eq expected.to_json
783-
end
784-
end
785-
end
786-
787768
class FakeHttpResponse
788769
def initialize(code, body = nil, headers={})
789770
@code = code

0 commit comments

Comments
 (0)