MarkUs: Testing

PR #7668 — Achieving full test coverage for CriteriaController.

Context

CriteriaController manages grading rubrics: creating, updating, reordering, and deleting criteria that TAs use to evaluate student submissions. The controller had significant untested code paths, particularly around authorization, edge cases in reordering logic, and interactions with released results.

Test Strategy

RSpec controller tests for Rails follow a pattern: set up state, issue a request, assert on the response and side effects.

describe CriteriaController do
  let(:assignment) { create(:assignment) }
  let(:criterion)  { create(:rubric_criterion, assignment: assignment) }

  describe "#update" do
    context "when results have been released" do
      let!(:released_result) { create(:released_result, assignment: assignment) }

      it "rejects the update with 409 Conflict" do
        patch :update, params: { id: criterion.id, criterion: { max_mark: 10 } }
        expect(response).to have_http_status(:conflict)
      end
    end
  end
end

Factory design. Created a released_result factory that composes existing factories (submission, result, grouping) with the released_to_students: true flag. Factory composition is preferable to fixtures: factories express the minimal state needed for each test, making dependencies explicit.

Coverage Patterns

Authorization testing. Each endpoint is tested with three roles: students (should be denied), TAs (limited access), and instructors (full access). This ensures the authorize! calls in the controller correctly delegate to the policy layer.

Concurrency guard. Once grading results are released to students, modifying the rubric would invalidate existing grades. The controller checks for released results and returns HTTP 409 (Conflict) if modifications are attempted. This is an application-level optimistic concurrency check.

Debugging Approach

Several tests initially failed due to incorrect assumptions about MarkUs’s data model. The systematic approach:

1. Reproduce. Run the failing test in isolation (rspec spec/controllers/criteria_controller_spec.rb:42)
2. Inspect. Add binding.pry at the controller action entry point, examine the request params and database state
3. Trace. Follow the code path from controller action through authorization, model mutation, and response rendering
4. Fix. Correct the factory setup or assertion to match the actual behavior

The most common failure mode: factories creating records with default associations that don’t match the test’s intended state. The fix is always to make factory usage more explicit, specifying the exact relationships needed.

Lessons

Test coverage as documentation. Each test describes a contract: “when X is true, the system does Y.” For a controller used by thousands of students and hundreds of TAs, these contracts prevent silent regressions during maintenance.

Focused PRs. This PR touched only test files and factory definitions. Zero production code changes. This made review straightforward: the reviewer could evaluate test quality without also evaluating behavioral changes.