diff --git a/.mention-bot b/.mention-bot index 4a62d5bd4c..41627bdbd2 100644 --- a/.mention-bot +++ b/.mention-bot @@ -9,7 +9,7 @@ "Procfile", ".eslintrc", "config/**/*.rb", - "app/controlleers/educators/sessions_controller.rb", + "app/controllers/educators/sessions_controller.rb", "lib/ldap_authenticatable_tiny/*.rb", "app/lib/shs_tiers.rb", "spec/lib/route_guards_spec.rb" diff --git a/.travis.yml b/.travis.yml index abe94325f8..f5c5214879 100644 --- a/.travis.yml +++ b/.travis.yml @@ -33,5 +33,5 @@ script: - ./scripts/ci/detect_package_lock.sh - rubocop - bundle exec brakeman -z - - bundle exec rspec spec + - ENABLE_RSPEC_COVERAGE_CHECKER=true bundle exec rspec spec - yarn test-cli diff --git a/spec/charts/student_profile_chart_spec.rb b/spec/charts/student_profile_chart_spec.rb index bb03801df0..947857c2d5 100644 --- a/spec/charts/student_profile_chart_spec.rb +++ b/spec/charts/student_profile_chart_spec.rb @@ -19,13 +19,13 @@ end it 'filters assessments with null SGP' do - with_growth_percentile = FactoryBot.create(:student_assessment, { + FactoryBot.create(:student_assessment, { student: student, scale_score: 504, growth_percentile: 26, assessment: Assessment.find_by(family: 'Next Gen MCAS', subject: 'Mathematics') }) - without_growth_percentile = FactoryBot.create(:student_assessment, { + FactoryBot.create(:student_assessment, { student: student, scale_score: 502, growth_percentile: nil, diff --git a/spec/coverage_bot.yml b/spec/coverage_bot.yml new file mode 100644 index 0000000000..c2a998f88f --- /dev/null +++ b/spec/coverage_bot.yml @@ -0,0 +1,7 @@ +# See rails_helpers.rb and coverage_checker.rb +# This is our own file, which we use to fail the build if specific files fall do not have 100% test coverage. +check_test_coverage_for_files: + - app/controllers/educators/sessions_controller.rb + - lib/authorizer.rb + - lib/ldap_authenticatable_tiny/model.rb + - lib/ldap_authenticatable_tiny/strategy.rb \ No newline at end of file diff --git a/spec/lib/authorizer_spec.rb b/spec/lib/authorizer_spec.rb index 7bd0301f77..36e2a4c83e 100644 --- a/spec/lib/authorizer_spec.rb +++ b/spec/lib/authorizer_spec.rb @@ -366,6 +366,40 @@ def authorized_students_when_viewing_as(educator, view_as_educator) end end + describe '#is_authorized_for_section?' do + let!(:test_section) { pals.shs_tuesday_biology_section } + + it 'allows access to own sections' do + expect(Authorizer.new(pals.shs_bill_nye).is_authorized_for_section?(test_section)).to eq true + end + + it 'allows districtwide access' do + expect(Authorizer.new(pals.uri).is_authorized_for_section?(test_section)).to eq true + expect(Authorizer.new(pals.rich_districtwide).is_authorized_for_section?(test_section)).to eq true + end + + it 'allows schoolwide access' do + expect(Authorizer.new(pals.shs_fatima_science_teacher).is_authorized_for_section?(test_section)).to eq true + expect(Authorizer.new(pals.shs_sofia_counselor).is_authorized_for_section?(test_section)).to eq true + expect(Authorizer.new(pals.shs_harry_housemaster).is_authorized_for_section?(test_section)).to eq true + end + + it 'guards access from others reading' do + unauthorized_educators = Educator.all - [ + pals.shs_bill_nye, + pals.uri, + pals.rich_districtwide, + pals.shs_fatima_science_teacher, + pals.shs_sofia_counselor, + pals.shs_harry_housemaster + ] + expect(unauthorized_educators.size).to be >= 1 + unauthorized_educators.each do |educator| + expect(Authorizer.new(educator).is_authorized_for_section?(test_section)).to eq(false), "failed for #{educator.email}" + end + end + end + describe '#is_authorized_to_write_transition_notes?' do it 'only allows K8 counselor with label and access to restricted notes' do expect(Authorizer.new(pals.west_counselor).is_authorized_to_write_transition_notes?).to eq true @@ -382,7 +416,7 @@ def authorized_students_when_viewing_as(educator, view_as_educator) expect(Authorizer.new(educator).is_authorized_for_deprecated_transition_note_ui?).to eq true end (Educator.all - authorized_educators).each do |educator| - expect(Authorizer.new(educator).is_authorized_to_write_transition_notes?).to eq false + expect(Authorizer.new(educator).is_authorized_for_deprecated_transition_note_ui?).to eq false end end end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 41abd8db39..d3c033f6b6 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -1,7 +1,6 @@ -# ---- Student Insights ---- +# This can't be moved and has to be run first. See https://github.com/colszowka/simplecov#troubleshooting require 'simplecov' SimpleCov.start -# --- end Student Insights # This file is copied to spec/ when you run 'rails generate rspec:install' ENV['RAILS_ENV'] ||= 'test' @@ -85,3 +84,6 @@ end end end + +# Test coverage checker +CoverageChecker.new.setup! diff --git a/spec/support/coverage_checker.rb b/spec/support/coverage_checker.rb new file mode 100644 index 0000000000..8ab6fb4994 --- /dev/null +++ b/spec/support/coverage_checker.rb @@ -0,0 +1,43 @@ +class CoverageChecker + ERROR_STATUS_CODE = 172 + + # By default, only run this on full test runs (eg, in Travis) or when explicitly asked. + # In the common development case where you only run a subset of tests, this will have + # frequent false positive failures (since only some of the tests were run). + def setup! + return unless EnvironmentVariable.is_true('ENABLE_RSPEC_COVERAGE_CHECKER') + SimpleCov.at_exit do + SimpleCov.result.format! + fail_if_uncovered!(SimpleCov.result.files) + end + end + + private + def fail_if_uncovered!(files) + files_to_check = filtered_files(files) + uncovered_files = files_to_check.select {|file| file.covered_percent < 100 } + + if files_to_check.size == 0 + puts "CoverageChecker: On this run, found no files to check for full coverage, skipping..." + elsif uncovered_files.size == 0 + puts "CoverageChecker: On this run, checked #{files_to_check.size} files for full coverage, all passed!" + else + puts "CoverageChecker: On this run, checked #{files_to_check.size} files for full coverage." + puts "CoverageChecker: Found #{uncovered_files.size} #{uncovered_files.size == 1 ? 'file that was not' : 'files that were not'} fully covered." + file_lines = uncovered_files.map do |file| + "#{file.filename} - #{file.covered_percent.round}%" + end + puts " - " + file_lines.join("\n - ") + puts "CoverageChecked: Exiting with error status #{ERROR_STATUS_CODE}" + Kernel.exit ERROR_STATUS_CODE + end + end + + def filtered_files(files) + config_file = File.open(File.join(Rails.root, 'spec', 'coverage_bot.yml')) + files_to_check = YAML.load(config_file)['check_test_coverage_for_files'] + files.select do |file| + files_to_check.any? {|file_to_check| file.filename.ends_with?(file_to_check)} + end + end +end