ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

rubocop: FactoryBot 

[skip-stages=Flakey]

Change-Id: Ia33d86601f9511dfdfd623ab39c15c007c829691
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/341935
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Aaron Ogata <aogata@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
This commit is contained in:
Cody Cutrer 2024-03-01 14:42:13 -07:00
parent bc3a49a39b
commit cb3e200ffd
13 changed files with 85 additions and 89 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
.rubocop.yml
View File

@ -22,6 +22,9 @@ Bundler:
Capybara:
Enabled: false # we don't use capybara (it's automatically included by rubocop-rspec until they hit 3.0)
FactoryBot:
Severity: error
Gemspec:
Severity: error
Gemspec/RequireMFA:
@ -221,13 +224,6 @@ Style/WordArray:
# them to the section above)
# auto-correct needs to be disabled for any that support it in the meantime
FactoryBot/ConsistentParenthesesStyle:
AutoCorrect: false
FactoryBot/CreateList:
AutoCorrect: false
FactoryBot/SyntaxMethods:
AutoCorrect: false
GraphQL/FieldMethod:
AutoCorrect: false
GraphQL/FieldName:

36
spec/apis/v1/conditional_release/rules_api_spec.rb
View File

@ -43,17 +43,17 @@ module ConditionalRelease
describe "GET index" do
before(:once) do
create :rule_with_scoring_ranges,
create(:rule_with_scoring_ranges,
course: @course,
trigger_assignment: @assignment
create :rule_with_scoring_ranges,
trigger_assignment: @assignment)
create(:rule_with_scoring_ranges,
course: @course,
trigger_assignment: @assignment,
assignment_count: 0
create :rule_with_scoring_ranges, course: @course
assignment_count: 0)
create(:rule_with_scoring_ranges, course: @course)
other_course = Course.create!
create :rule_with_scoring_ranges, course: other_course
create(:rule_with_scoring_ranges, course: other_course)
@url = "/api/v1/courses/#{@course.id}/mastery_paths/rules"
@base_params = {
@ -107,11 +107,11 @@ module ConditionalRelease
describe "GET show" do
before :once do
@rule = create :rule_with_scoring_ranges,
@rule = create(:rule_with_scoring_ranges,
course: @course,
scoring_range_count: 2,
assignment_set_count: 2,
assignment_count: 3
assignment_count: 3)
@url = "/api/v1/courses/#{@course.id}/mastery_paths/rules/#{@rule.id}"
@base_params = {
@ -247,7 +247,7 @@ module ConditionalRelease
describe "PUT update" do
before :once do
@rule = create :rule, course: @course, trigger_assignment: @assignment
@rule = create(:rule, course: @course, trigger_assignment: @assignment)
@url = "/api/v1/courses/#{@course.id}/mastery_paths/rules/#{@rule.id}"
@other_assignment = @course.assignments.create!
@base_params = {
@ -283,10 +283,10 @@ module ConditionalRelease
end
it "updates with scoring ranges" do
rule = create :rule_with_scoring_ranges,
rule = create(:rule_with_scoring_ranges,
course: @course,
scoring_range_count: 2,
assignment_count: 3
assignment_count: 3)
range = rule.scoring_ranges[0]
range.upper_bound = 99
rule_params = rule.as_json(include: :scoring_ranges, include_root: false)
@ -305,9 +305,9 @@ module ConditionalRelease
end
it "updates removes scoring ranges" do
rule = create :rule_with_scoring_ranges,
rule = create(:rule_with_scoring_ranges,
course: @course,
scoring_range_count: 2
scoring_range_count: 2)
rule_params = rule.as_json(include: :scoring_ranges, include_root: false)
rule_params["scoring_ranges"].shift
@ -322,11 +322,11 @@ module ConditionalRelease
end
it "updates with assignments in order" do
rule = create :rule_with_scoring_ranges,
rule = create(:rule_with_scoring_ranges,
course: @course,
scoring_range_count: 2,
assignment_set_count: 2,
assignment_count: 1
assignment_count: 1)
rule_params = rule.as_json(include: { scoring_ranges: { include: { assignment_sets: { include: :assignment_set_associations } } } }, include_root: false)
changed_assignment = @course.assignments.create!
@ -357,11 +357,11 @@ module ConditionalRelease
end
it "updates with assignments in rearranged order" do
rule = create :rule_with_scoring_ranges,
rule = create(:rule_with_scoring_ranges,
course: @course,
scoring_range_count: 1,
assignment_set_count: 1,
assignment_count: 3
assignment_count: 3)
rule_params = rule.as_json(include_root: false, include: { scoring_ranges: { include: { assignment_sets: { include: :assignment_set_associations } } } })
assignments = rule_params["scoring_ranges"][0]["assignment_sets"][0]["assignment_set_associations"]
@ -387,7 +387,7 @@ module ConditionalRelease
describe "DELETE destroy" do
before :once do
@rule = create :rule, course: @course
@rule = create(:rule, course: @course)
@url = "/api/v1/courses/#{@course.id}/mastery_paths/rules/#{@rule.id}"
@base_params = {
controller: "conditional_release/rules",

2
spec/apis/v1/conditional_release/stats_api_spec.rb
View File

@ -33,7 +33,7 @@ module ConditionalRelease
context "rules stats" do
before(:once) do
@rule = create :rule, course: @course
@rule = create(:rule, course: @course)
end
describe "GET students_per_range" do

4
spec/conditional_release_spec_helper.rb
View File

@ -32,14 +32,14 @@ RSpec.shared_examples "a soft-deletable model" do
end
it "soft deletes" do
instance = create described_class.name.underscore.sub("conditional_release/", "").to_sym
instance = create(described_class.name.underscore.sub("conditional_release/", "").to_sym)
instance.destroy!
expect(described_class.exists?(instance.id)).to be true
expect(described_class.active.exists?(instance.id)).to be false
end
it "allows duplicates on unique attributes when one instance is soft deleted" do
instance = create described_class.name.underscore.sub("conditional_release/", "").to_sym
instance = create(described_class.name.underscore.sub("conditional_release/", "").to_sym)
copy = instance.clone
instance.destroy!
expect { copy.save! }.to_not raise_error

2
spec/factory_bot/conditional_release/assignment_sets.rb
View File

@ -28,7 +28,7 @@ FactoryBot.define do
end
after(:create) do |assignment_set, evaluator|
create_list :assignment_set_association, evaluator.assignment_count, assignment_set:
create_list(:assignment_set_association, evaluator.assignment_count, assignment_set:)
end
end
end

4
spec/factory_bot/conditional_release/scoring_ranges.rb
View File

@ -31,10 +31,10 @@ FactoryBot.define do
end
after(:create) do |range, evaluator|
create_list :assignment_set_with_assignments,
create_list(:assignment_set_with_assignments,
evaluator.assignment_set_count,
scoring_range: range,
assignment_count: evaluator.assignment_count
assignment_count: evaluator.assignment_count)
end
end
end

28
spec/models/conditional_release/assignment_set_action_spec.rb
View File

@ -25,9 +25,9 @@ module ConditionalRelease
it_behaves_like "a soft-deletable model"
it "must have student_id and actor_id" do
set = create :assignment_set
set = create(:assignment_set)
[:student_id, :actor_id].each do |attr|
action = build :assignment_set_action, assignment_set: set
action = build(:assignment_set_action, assignment_set: set)
action.send(:"#{attr}=", nil)
expect(action.valid?).to be false
action.send(:"#{attr}=", "")
@ -38,8 +38,8 @@ module ConditionalRelease
end
it "must have action" do
set = create :assignment_set
action = build :assignment_set_action, assignment_set: set
set = create(:assignment_set)
action = build(:assignment_set_action, assignment_set: set)
action.action = nil
expect(action.valid?).to be false
action.action = ""
@ -49,8 +49,8 @@ module ConditionalRelease
end
it "must have source" do
set = create :assignment_set
action = build :assignment_set_action, assignment_set: set
set = create(:assignment_set)
action = build(:assignment_set_action, assignment_set: set)
action.source = nil
expect(action.valid?).to be false
action.source = ""
@ -60,8 +60,8 @@ module ConditionalRelease
end
it "must have an assignment_set_id" do
set = create :assignment_set
action = build :assignment_set_action
set = create(:assignment_set)
action = build(:assignment_set_action)
action.assignment_set_id = nil
expect(action.valid?).to be false
action.assignment_set_id = set.id
@ -69,7 +69,7 @@ module ConditionalRelease
end
it "is valid when assignment_set does not exist" do
action = create :assignment_set_action
action = create(:assignment_set_action)
set_id = action.assignment_set.id
action.assignment_set.destroy!
expect(action.reload.valid?).to be true
@ -80,9 +80,9 @@ module ConditionalRelease
it "selects only the most recent Action for each Set and user_id" do
actions = []
actions << create(:assignment_set_action, student_id: 2, assignment_set: create(:assignment_set))
set = create :assignment_set
set = create(:assignment_set)
actions << create(:assignment_set_action, student_id: 1, assignment_set: set)
actions.concat Array.new(2) { create :assignment_set_action, student_id: 2, assignment_set: set }
actions.concat create_list(:assignment_set_action, 2, student_id: 2, assignment_set: set)
actions.last.update_attribute(:created_at, 1.hour.ago)
expect(AssignmentSetAction.latest).to eq actions[0..2]
end
@ -90,7 +90,7 @@ module ConditionalRelease
describe "self.current_assignments" do
it "selects only actions that have not been unassigned" do
set = create :assignment_set
set = create(:assignment_set)
create(:assignment_set_action, action: "assign", student_id: 1, assignment_set: set, created_at: 1.hour.ago)
create(:assignment_set_action, action: "unassign", student_id: 1, assignment_set: set)
expect(AssignmentSetAction.current_assignments("user")).to eq []
@ -99,7 +99,7 @@ module ConditionalRelease
end
it "selects only actions for the specified sets" do
actions = Array.new(3) { create(:assignment_set_action, student_id: 1) }
actions = create_list(:assignment_set_action, 3, student_id: 1)
selected_sets = actions[1..2].map(&:assignment_set)
expect(AssignmentSetAction.current_assignments(1, selected_sets).order(:id)).to eq actions[1..2]
end
@ -107,7 +107,7 @@ module ConditionalRelease
describe "self.create_from_sets" do
it "creates records" do
range = create :scoring_range_with_assignments, assignment_set_count: 4
range = create(:scoring_range_with_assignments, assignment_set_count: 4)
assigned = range.assignment_sets[0..1]
unassigned = range.assignment_sets[2..3]
audit_opts = { student_id: 1, actor_id: 2, source: "grade_change" }

8
spec/models/conditional_release/assignment_set_association_spec.rb
View File

@ -25,14 +25,14 @@ module ConditionalRelease
it_behaves_like "a soft-deletable model"
it "must have an assignment_id" do
assignment = build :assignment_set_association
assignment = build(:assignment_set_association)
assignment.assignment_id = nil
expect(assignment.valid?).to be false
end
it "enforces unique assignment_id in assignment_set" do
asg = create :assignment_set_association
dup = build :assignment_set_association, assignment_id: asg.assignment_id
asg = create(:assignment_set_association)
dup = build(:assignment_set_association, assignment_id: asg.assignment_id)
asg.assignment_set.assignment_set_associations << dup
expect(dup.valid?).to be false
expect(dup.errors["assignment_id"].to_s).to match(/taken/)
@ -41,7 +41,7 @@ module ConditionalRelease
end
it "enforces not having the same assigment_id as the trigger_assignment of its rule" do
asg = create :assignment_set_association
asg = create(:assignment_set_association)
asg.assignment_id = asg.rule.trigger_assignment_id
expect(asg.valid?).to be false
expect(asg.errors["assignment_id"].to_s).to match(/trigger/)

2
spec/models/conditional_release/assignment_set_spec.rb
View File

@ -25,7 +25,7 @@ module ConditionalRelease
it_behaves_like "a soft-deletable model"
it "must have a scoring_range_id" do
assignment_set = build :assignment_set, scoring_range_id: nil
assignment_set = build(:assignment_set, scoring_range_id: nil)
expect(assignment_set.valid?).to be false
end
end

34
spec/models/conditional_release/rule_spec.rb
View File

@ -26,7 +26,7 @@ module ConditionalRelease
describe "rule definition" do
it "must have a root account id" do
rule = build :rule
rule = build(:rule)
rule.root_account_id = nil
expect(rule.valid?).to be false
rule.root_account_id = ""
@ -36,32 +36,32 @@ module ConditionalRelease
describe "assignment_sets_for_score" do
before :once do
@rule = create :rule
create :scoring_range_with_assignments,
@rule = create(:rule)
create(:scoring_range_with_assignments,
rule: @rule,
lower_bound: 90,
upper_bound: nil,
assignment_set_count: 1
create :scoring_range_with_assignments,
assignment_set_count: 1)
create(:scoring_range_with_assignments,
rule: @rule,
lower_bound: 70,
upper_bound: 90,
assignment_set_count: 1
create :scoring_range_with_assignments,
assignment_set_count: 1)
create(:scoring_range_with_assignments,
rule: @rule,
lower_bound: 50,
upper_bound: 70,
assignment_set_count: 1
assignment_set_count: 1)
end
it "must apply all scoring ranges" do
expect(@rule.assignment_sets_for_score(91).length).to eq(1)
# create a range that crosses the ranges above
create :scoring_range_with_assignments,
create(:scoring_range_with_assignments,
rule: @rule,
lower_bound: 80,
upper_bound: 95,
assignment_set_count: 2
assignment_set_count: 2)
expect(@rule.assignment_sets_for_score(91).length).to eq(3)
end
@ -70,22 +70,22 @@ module ConditionalRelease
end
it "must return [] if no scoring ranges are defined" do
rule = create :rule
rule = create(:rule)
expect(rule.assignment_sets_for_score(10)).to eq([])
end
end
describe "with_assignments" do
before do
@rule1 = create :rule_with_scoring_ranges
@rule1 = create(:rule_with_scoring_ranges)
@rule1.scoring_ranges.last.assignment_sets.destroy_all
@rule2 = create :rule_with_scoring_ranges, assignment_set_count: 0
@rule3 = create :rule_with_scoring_ranges, assignment_count: 0
@rule4 = create :rule_with_scoring_ranges,
@rule2 = create(:rule_with_scoring_ranges, assignment_set_count: 0)
@rule3 = create(:rule_with_scoring_ranges, assignment_count: 0)
@rule4 = create(:rule_with_scoring_ranges,
scoring_range_count: 1,
assignment_set_count: 1,
assignment_count: 1
@rule5 = create :rule
assignment_count: 1)
@rule5 = create(:rule)
end
let(:rules) { Rule.with_assignments.to_a }

28
spec/models/conditional_release/scoring_range_spec.rb
View File

@ -26,20 +26,20 @@ module ConditionalRelease
describe "scoring range definition" do
it "must have at least one bound" do
range = build :scoring_range
range = build(:scoring_range)
range.lower_bound = range.upper_bound = nil
expect(range.valid?).to be false
end
it "must have lower bound less than upper" do
range = build :scoring_range
range = build(:scoring_range)
range.lower_bound = 100
range.upper_bound = 1
expect(range.valid?).to be false
end
it "can have null bounds" do
range = build :scoring_range
range = build(:scoring_range)
range.lower_bound = nil
range.upper_bound = 100
expect(range.valid?).to be true
@ -49,10 +49,10 @@ module ConditionalRelease
end
it "must have non-negative bounds" do
range = build :scoring_range
range = build(:scoring_range)
range.lower_bound = -10
expect(range.valid?).to be false
range = build :scoring_range
range = build(:scoring_range)
range.upper_bound = -10
expect(range.valid?).to be false
end
@ -60,9 +60,9 @@ module ConditionalRelease
describe "for_score" do
before do
@rule = create :rule
@range = create :scoring_range, rule: @rule
create :assignment_set_association, scoring_range: @range
@rule = create(:rule)
@range = create(:scoring_range, rule: @rule)
create(:assignment_set_association, scoring_range: @range)
end
it "must return an empty relation when nothing matches" do
@ -115,16 +115,16 @@ module ConditionalRelease
describe "contains_score" do
before do
@rule = create :rule
@range = create :scoring_range, rule: @rule
create :assignment_set_association, scoring_range: @range
@rule = create(:rule)
@range = create(:scoring_range, rule: @rule)
create(:assignment_set_association, scoring_range: @range)
end
it "must properly evaluate a bound of 0" do
@range.lower_bound = nil
@range.upper_bound = 0
@range.save!
range2 = create :scoring_range, rule: @rule
range2 = create(:scoring_range, rule: @rule)
range2.lower_bound = 0
range2.upper_bound = 1
range2.save!
@ -143,13 +143,13 @@ module ConditionalRelease
describe "#assignment_sets" do
it "builds a assignment_set if one does not exist" do
range = create :scoring_range_with_assignments, assignment_set_count: 0
range = create(:scoring_range_with_assignments, assignment_set_count: 0)
expect(AssignmentSet.count).to eq 0
expect(range.assignment_sets.length).to eq 1
end
it "returns existing assignment_sets" do
range = create :scoring_range_with_assignments, assignment_set_count: 2
range = create(:scoring_range_with_assignments, assignment_set_count: 2)
expect(range.assignment_sets.length).to eq 2
end
end

10
spec/models/conditional_release/stats_spec.rb
View File

@ -24,10 +24,10 @@ module ConditionalRelease
before :once do
@course = course_factory(active_all: true)
@students = n_students_in_course(4, course: @course)
@rule = create :rule, course: @course
@sr1 = create :scoring_range_with_assignments, rule: @rule, upper_bound: nil, lower_bound: 0.7, assignment_set_count: 2, assignment_count: 5
@sr2 = create :scoring_range_with_assignments, rule: @rule, upper_bound: 0.7, lower_bound: 0.4, assignment_set_count: 2, assignment_count: 5
@sr3 = create :scoring_range_with_assignments, rule: @rule, upper_bound: 0.4, lower_bound: nil, assignment_set_count: 2, assignment_count: 5
@rule = create(:rule, course: @course)
@sr1 = create(:scoring_range_with_assignments, rule: @rule, upper_bound: nil, lower_bound: 0.7, assignment_set_count: 2, assignment_count: 5)
@sr2 = create(:scoring_range_with_assignments, rule: @rule, upper_bound: 0.7, lower_bound: 0.4, assignment_set_count: 2, assignment_count: 5)
@sr3 = create(:scoring_range_with_assignments, rule: @rule, upper_bound: 0.4, lower_bound: nil, assignment_set_count: 2, assignment_count: 5)
@as1 = @sr1.assignment_sets.first
@as2 = @sr2.assignment_sets.first
@ -227,7 +227,7 @@ module ConditionalRelease
context "trends per assignment" do
before do
@rule.scoring_ranges.destroy_all
@sr = create :scoring_range_with_assignments, assignment_count: 1, rule: @rule, upper_bound: nil, lower_bound: 0
@sr = create(:scoring_range_with_assignments, assignment_count: 1, rule: @rule, upper_bound: nil, lower_bound: 0)
@trigger = @rule.trigger_assignment
@follow_on = @sr.assignment_set_associations.first.assignment
end

6
spec/selenium/conditional_release/conditional_release_spec.rb
View File

@ -217,21 +217,21 @@ describe "native canvas conditional release" do
before do
@trigger_assmt = @course.assignments.create!(points_possible: 10, submission_types: "online_text_entry")
ranges = [
FactoryBot.create(
create(
:scoring_range_with_assignments,
assignment_set_count: 1,
assignment_count: 1,
lower_bound: 0.7,
upper_bound: 1.0
),
FactoryBot.create(
create(
:scoring_range_with_assignments,
assignment_set_count: 1,
assignment_count: 2,
lower_bound: 0.4,
upper_bound: 0.7
),
FactoryBot.create(
create(
:scoring_range_with_assignments,
assignment_set_count: 2,
assignment_count: 2,