Merge pull request #6239 from mishaschwartz/v2.1.3

V2.1.3

Changelog.md

@@ -1,5 +1,8 @@
 # Changelog
 
+## [v2.1.3]
+- Fix bug where automated test results were occasionally associated with the wrong grouping (#6238)
+
 ## [v2.1.2]
 - Fix bug preventing use of the update_grades API route (#6188)
 - Fix overly restrictive policies for admin users (#6209)

app/MARKUS_VERSION

@@ -1 +1 @@
-VERSION=v2.1.2,PATCH_LEVEL=DEV
+VERSION=v2.1.3,PATCH_LEVEL=DEV

app/helpers/automated_tests_helper.rb

@@ -188,10 +188,16 @@ def run_tests(assignment, host_with_port, group_ids, role, collected: true, batc
       req = Net::HTTP::Put.new(uri)
       set_headers(req, assignment.course.autotest_setting.api_key)
       markus_address = get_markus_address(host_with_port)
+      # NOTE: this stores the order in which groups are sent to the autotester. The autotester will return an array of
+      #       autotest_test_ids *in the same order* so this array can be zipped together with that one to create a
+      #       mapping between the group id and the autotest_test_id (used later to associate the result of the test to
+      #       a test run).
+      group_id_send_order = []
       test_data = group_info_for_tests(assignment, group_ids).map do |id_, name, starter_files|
         param = collected ? 'collected=true' : ''
         file_url = "#{markus_address}/api/courses/#{assignment.course.id}/assignments/#{assignment.id}/" \
                    "groups/#{id_}/submission_files?#{param}"
+        group_id_send_order << id_
         # TODO: add other relevant info to env_vars as needed and make the environment variable customizable
         { file_url: file_url, env_vars: { MARKUS_GROUP: name, MARKUS_STARTER_FILES: starter_files.to_json } }
       end
@@ -202,7 +208,7 @@ def run_tests(assignment, host_with_port, group_ids, role, collected: true, batc
       }.to_json
       res = send_request!(req, uri)
       autotest_test_ids = JSON.parse(res.body)['test_ids']
-      test_id_hash = group_ids.zip(autotest_test_ids).to_h
+      test_id_hash = group_id_send_order.zip(autotest_test_ids).to_h
       groupings = Grouping.includes(:current_submission_used).where(group_id: group_ids, assignment: assignment)
       groupings.each do |grouping|
         revision_id = collected ? nil : grouping.access_repo { |repo| repo.get_latest_revision.revision_identifier }
@@ -327,6 +333,8 @@ def add_feedback_data(results, settings_id, test_id, autotest_setting)
   # and the given group.
   #
   # Only groups with id in +group_ids+ are included in the list.
+  # Note that the group information returned by this function is not guaranteed to be in
+  # the same order as +group_ids+.
   def group_info_for_tests(assignment, group_ids)
     assignment.groupings
               .left_outer_joins(:group, starter_file_entries: :starter_file_group)

spec/jobs/autotest_run_job_spec.rb

@@ -67,6 +67,12 @@
           subject
           expect(TestRun.where(status: :in_progress).count).to eq n_groups
         end
+        it 'should create test runs associated to the correct grouping and with the correct autotest_run_ids' do
+          subject
+          test_run_data = TestRun.joins(grouping: :group).pluck('groups.id', 'test_runs.autotest_test_id')
+          expected = groupings.map { |g| g.group.id }.zip(JSON.parse(dummy_return.body)['test_ids'])
+          expect(test_run_data).to contain_exactly(*expected)
+        end
       end
       it 'should enqueue an AutotestResultsJob job' do
         expect(AutotestResultsJob).to receive(:perform_later).once