Skip to content

Commit

Permalink
fix: ensure broken aux file does not kill future pdf generation
Browse files Browse the repository at this point in the history
  • Loading branch information
macite committed Oct 25, 2024
1 parent c19e149 commit 715ccaf
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 2 deletions.
10 changes: 8 additions & 2 deletions app/models/task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1167,8 +1167,11 @@ def convert_submission_to_pdf(source_folder: FileHelper.student_work_dir(:new),
begin
pdf_text = tac.make_pdf
rescue => e
# Try again... with convert to ascic
#
# Try again...
# Without newpax
# Ensure latex aux file is removed
Dir.glob(Rails.root.join('tmp/rails-latex/**/input.aux')).each { |f| File.delete(f) }

tac2 = TaskAppController.new
tac2.init(self, true)

Expand Down Expand Up @@ -1234,6 +1237,9 @@ def convert_submission_to_pdf(source_folder: FileHelper.student_work_dir(:new),
add_text_comment project.tutor_for(task_definition), "**Automated Comment**: Something went wrong with your submission. Check the files and resubmit this task. #{e.message}"
raise e
ensure
# Ensure latex aux file is removed - if broken will cause issues for next submission in sidekiq
Dir.glob(Rails.root.join('tmp/rails-latex/**/input.aux')).each { |f| File.delete(f) }

clear_in_process
end
end
Expand Down
52 changes: 52 additions & 0 deletions test/models/task_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -757,6 +757,58 @@ def test_pdf_creation_fails_on_invalid_pdf
end
end

def test_pax_crash_does_not_stop_pdf_creation
unit = FactoryBot.create(:unit, student_count: 1, task_count: 0)
td = TaskDefinition.new({
unit_id: unit.id,
tutorial_stream: unit.tutorial_streams.first,
name: 'PDF Test Task',
description: 'Test task',
weighting: 4,
target_grade: 0,
start_date: unit.start_date + 1.week,
target_date: unit.start_date + 2.weeks,
abbreviation: 'PDFTestTask',
restrict_status_updates: false,
upload_requirements: [ { "key" => 'file0', "name" => 'A pdf file', "type" => 'document' } ],
plagiarism_warn_pct: 0.8,
is_graded: false,
max_quality_pts: 0
})
td.save!

data_to_post = {
trigger: 'ready_for_feedback'
}

# submit an encrypted (but valid) PDF file and ensure it's rejected immediately
data_to_post = with_file('test_files/submissions/valid.pdf', 'application/json', data_to_post)

project = unit.active_projects.first

add_auth_header_for user: unit.main_convenor_user

post "/api/projects/#{project.id}/task_def_id/#{td.id}/submission", data_to_post

assert_equal 201, last_response.status, last_response_body

task = project.task_for_task_definition(td)

rails_latex_path = Rails.root.join("tmp/rails-latex/#{Process.pid}-#{Thread.current.hash}")
FileUtils.mkdir_p(rails_latex_path)
FileUtils.cp(Rails.root.join('test_files/latex/input-broken.aux'), "#{rails_latex_path}/input.aux")

assert task.convert_submission_to_pdf(log_to_stdout: false)
path = task.zip_file_path_for_done_task
assert path
assert File.exist? path
assert File.exist? task.final_pdf_path

td.destroy
assert_not File.exist? path
unit.destroy!
end

def test_accept_files_checks_they_all_exist
project = FactoryBot.create(:project)
unit = project.unit
Expand Down
40 changes: 40 additions & 0 deletions test_files/latex/input-broken.aux
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
\relax
\providecommand\hyper@newdestlabel[2]{}
\providecommand\HyField@AuxAddToFields[1]{}
\providecommand\HyField@AuxAddToCoFields[2]{}
\providecommand{\NEWPAX@DestReq}[2]{}
\providecommand{\NEWPAX@DestProv}[2]{}
\providecommand\BKM@entry[2]{}
\NEWPAX@DestProv{000-document.newpax}{Congratulations!}
\NEWPAX@DestProv{000-document.newpax}{Decision-Tree}
\NEWPAX@DestProv{000-document.newpax}{Explanation-using-LIME}
\NEWPAX@DestProv{000-document.newpax}{Feature-importance:}
\NEWPAX@DestProv{000-document.newpax}{LIME-for-explaining-prediction-images}
\NEWPAX@DestProv{000-document.newpax}{LIME:}
\NEWPAX@DestProv{000-document.newpax}{Model-Interpretation-Methods}
\NEWPAX@DestProv{000-document.newpax}{Model-Performance}
\NEWPAX@DestProv{000-document.newpax}{Pre-processing}
\NEWPAX@DestProv{000-document.newpax}{Predictions-on-the-test-data}
\NEWPAX@DestProv{000-document.newpax}{Split-Train-and-Test-Datasets}
\NEWPAX@DestProv{000-document.newpax}{Training-the-classification-model}
\NEWPAX@DestProv{000-document.newpax}{Understanding-the-Census-Income-Dataset}
\NEWPAX@DestProv{000-document.newpax}{Visualzing-the-Tree}
\NEWPAX@DestProv{000-document.newpax}{When-a-person's-income-%3C=-$50K}
\NEWPAX@DestProv{000-document.newpax}{When-a-person's-income-%3E-$50K}
\NEWPAX@DestProv{000-document.newpax}{When-a-person's-income-actual-is-different-than-predicted}
\NEWPAX@DestProv{000-document.newpax}{Your-own-test-example}
\NEWPAX@DestProv{000-document.newpax}{sk-container-id-1}
\NEWPAX@DestProv{000-document.newpax}{sk-container-id-2}
\NEWPAX@DestProv{000-document.newpax}{sk-estimator-id-1}
\NEWPAX@DestProv{000-document.newpax}{sk-estimator-id-2}
\NEWPAX@DestProv{000-document.newpax}{top_div8WD77G29ZHEFM8A}
\NEWPAX@DestProv{000-document.newpax}{top_divDR5YA6I6PZOKFNQ}
\NEWPAX@DestProv{000-document.newpax}{top_divGH11HTIE1SXJUDR}
\NEWPAX@DestProv{000-document.newpax}{top_divQ4ALLQ5KXQWP33K}
\NEWPAX@DestProv{000-document.newpax}{top_divR1OKF6JPUWMW89V}
\NEWPAX@DestProv{000-document.newpax}{top_divSJB1S48PH4T84MA}
\NEWPAX@DestProv{000-document.newpax}{top_divWA6RJ9LWIJMH2XT}
\newlabel{LastPage}{{}{27}{}{page.27}{}}
\gdef\lastpage@lastpage{27}
\gdef\lastpage@lastpageHy{27}
\gdef \@abspage@last{28}

0 comments on commit 715ccaf

Please sign in to comment.