autolab / Autolab

Course management service that enables auto-graded programming assignments.
http://www.autolabproject.com/
Apache License 2.0
764 stars 219 forks source link

Assessment config file serialization should use stable key ordering #1627

Closed zackw closed 1 year ago

zackw commented 2 years ago

Your environment Carnegie Mellon's Autolab at https://autolab.andrew.cmu.edu/

Autolab v2.8.0 (Is the exact commit hash exposed anywhere on the website?)

Steps To Reproduce

  1. Import an assessment from a Git checkout.
  2. Make any change to the assessment configuration through the "edit assessment" screen
  3. git diff the assessment configuration file.
  4. In addition to the change you actually made, the diff will have a lot of "noise" changes because the YAML serializer doesn't always emit the keys of each mapping (Ruby hash) in the same order.

Example noisy diff

diff --git a/cprogramminglab.yml b/cprogramminglab.yml
index 22b8788..47cdb9d 100644
--- a/cprogramminglab.yml
+++ b/cprogramminglab.yml
@@ -1,17 +1,17 @@
 ---
 general:
-  display_name: C Programming Lab
   name: cprogramminglab
-  category_name: Lab
   description: Check out your C programming skills
+  display_name: C Programming Lab
+  handin_filename: cprogramminglab-handin.tar
+  handin_directory: handin
+  max_grace_days: 0
   handout: cprogramminglab-handout.tar
   writeup: writeup/cprogramminglab.pdf
-  handin_filename: cprogramminglab-handin.tar
-  max_size: 2
-  disable_handins: false
   max_submissions: -1
-  max_grace_days: 0
-  handin_directory: handin
+  disable_handins: false
+  max_size: 2
+  category_name: Lab
   embedded_quiz: false
 problems:
 - name: Trace-01
@@ -76,7 +76,7 @@ problems:
   optional: false
 autograder:
   autograde_timeout: 180
-  autograde_image: rhelbeta.img
+  autograde_image: ubun2022_testing.img
   release_score: true
 scoreboard:
   banner: ''

The first "hunk" of this diff is entirely due to reordering of mapping keys; nothing has actually changed. The second hunk shows the change I did make.

Desired diff for the same actual change

diff --git a/cprogramminglab.yml b/cprogramminglab.yml
index 22b8788..47cdb9d 100644
--- a/cprogramminglab.yml
+++ b/cprogramminglab.yml
@@ -76,7 +76,7 @@ problems:
   optional: false
 autograder:
   autograde_timeout: 180
-  autograde_image: rhelbeta.img
+  autograde_image: ubun2022_testing.img
   release_score: true
 scoreboard:
   banner: ''

Notes

I don't speak Ruby, but https://dzone.com/articles/generating-yaml-hashes-sorted looks like it could be useful as, at least, a starting point for the fix. It is not strictly necessary for the keys to be sorted; the important thing is for them to always be written out in the same order; but sorting them is probably the easiest way to achieve that.

damianhxy commented 1 year ago

The logic to update seems to be in assessment.rb#dump_yaml

damianhxy commented 1 year ago

Regarding your question about the commit hash, it is not exposed on the website.

The code running in production can be found on the fork https://github.com/cg2v/Autolab

The current code is on the branch https://github.com/cg2v/Autolab/commits/deploy-fall-2022-1

damianhxy commented 1 year ago

We are using the Psych Library for generating YAML files, and unfortunately it seems that it does not support a way to sort keys (https://github.com/ruby/psych/issues/542).

Possible workaround: https://stackoverflow.com/a/56137489