Skip to content

Commit 2a5c046

Browse files
HeliozoaHeliozoa
committed
Improve error handling when expected test result files are missing
1 parent f64a7f5 commit 2a5c046

File tree

8 files changed

+117
-53
lines changed

8 files changed

+117
-53
lines changed

‎CONTRIBUTING.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ Install [OpenJDK 11](https://openjdk.java.net/install/index.html) (other version
1212

1313
Install [.NET 5.0](https://dotnet.microsoft.com/download)
1414

15-
Install [check](https://libcheck.github.io/check/) (works with at least 0.14 and 0.15)
15+
Install [check](https://libcheck.github.io/check/) (works with at least 0.14 and 0.15) and [valgrind](https://valgrind.org/)
1616

1717
Install [R](https://www.r-project.org/), [devtools](https://devtools.r-lib.org/) with `install.packages("devtools")` and [tmc-r-tester](https://github.com/testmycode/tmc-rstudio) with `devtools::install_github("testmycode/tmc-r-tester/tmcRtestrunner", build = FALSE)`
1818

‎bindings/tmc-langs-node/package-lock.json‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎plugins/csharp/src/error.rs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,16 @@ pub enum CSharpError {
1616
CacheDir,
1717
#[error("Could not locate boostrap DLL at {0}")]
1818
MissingBootstrapDll(PathBuf),
19+
#[error(
20+
"Failed to locate test results at {path}
21+
stdout: {stdout}
22+
stderr: {stderr}"
23+
)]
24+
MissingTestResults {
25+
path: PathBuf,
26+
stdout: String,
27+
stderr: String,
28+
},
1929

2030
// Wrapping other error types.
2131
#[error("File IO error")]

‎plugins/csharp/src/plugin.rs‎

Lines changed: 33 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,7 @@ impl CSharpPlugin {
105105
/// Parses the test results JSON file at the path argument.
106106
fn parse_test_results(
107107
test_results_path: &Path,
108-
logs: HashMap<String, String>,
109-
) -> Result<RunResult, CSharpError> {
108+
) -> Result<(RunStatus, Vec<TestResult>), CSharpError> {
110109
log::debug!("parsing C# test results");
111110
let test_results = file_util::open_file(test_results_path)?;
112111
let test_results: Vec<CSTestResult> = serde_json::from_reader(test_results)
@@ -126,11 +125,7 @@ impl CSharpPlugin {
126125
.into_iter()
127126
.map(|t| t.into_test_result(&failed_points))
128127
.collect();
129-
Ok(RunResult {
130-
status,
131-
test_results,
132-
logs,
133-
})
128+
Ok((status, test_results))
134129
}
135130
}
136131

@@ -238,20 +233,41 @@ impl LanguagePlugin for CSharpPlugin {
238233
Ok(output) => {
239234
let stdout = String::from_utf8_lossy(&output.stdout);
240235
let stderr = String::from_utf8_lossy(&output.stderr);
241-
log::trace!("stdout: {}", stdout);
242-
log::debug!("stderr: {}", stderr);
243-
let mut logs = HashMap::new();
244-
logs.insert("stdout".to_string(), stdout.into_owned());
245-
logs.insert("stderr".to_string(), stderr.into_owned());
246-
247236
if !output.status.success() {
237+
log::warn!("stdout: {}", stdout);
238+
log::error!("stderr: {}", stderr);
239+
let mut logs = HashMap::new();
240+
logs.insert("stdout".to_string(), stdout.into_owned());
241+
logs.insert("stderr".to_string(), stderr.into_owned());
248242
return Ok(RunResult {
249243
status: RunStatus::CompileFailed,
250244
test_results: vec![],
251245
logs,
252246
});
253247
}
254-
Self::parse_test_results(&test_results_path, logs).map_err(|e| e.into())
248+
249+
log::trace!("stdout: {}", stdout);
250+
log::debug!("stderr: {}", stderr);
251+
252+
if !test_results_path.exists() {
253+
return Err(CSharpError::MissingTestResults {
254+
path: test_results_path,
255+
stdout: stdout.into_owned(),
256+
stderr: stderr.into_owned(),
257+
}
258+
.into());
259+
}
260+
let (status, test_results) = Self::parse_test_results(&test_results_path)?;
261+
file_util::remove_file(&test_results_path)?;
262+
263+
let mut logs = HashMap::new();
264+
logs.insert("stdout".to_string(), stdout.into_owned());
265+
logs.insert("stderr".to_string(), stderr.into_owned());
266+
Ok(RunResult {
267+
status,
268+
test_results,
269+
logs,
270+
})
255271
}
256272
Err(TmcError::Command(CommandError::TimeOut { stdout, stderr, .. })) => {
257273
let mut logs = HashMap::new();
@@ -477,9 +493,9 @@ mod test {
477493
]
478494
"#,
479495
);
480-
let parse = CSharpPlugin::parse_test_results(&json, HashMap::new()).unwrap();
481-
assert_eq!(parse.status, RunStatus::TestsFailed);
482-
assert_eq!(parse.test_results.len(), 2);
496+
let (status, test_results) = CSharpPlugin::parse_test_results(&json).unwrap();
497+
assert_eq!(status, RunStatus::TestsFailed);
498+
assert_eq!(test_results.len(), 2);
483499
}
484500

485501
#[test]

‎plugins/python3/src/error.rs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,16 @@ pub enum PythonError {
2424
UnexpectedHmac,
2525
#[error("Failed to verify the test results")]
2626
InvalidHmac,
27+
#[error(
28+
"Failed to locate test results at {path}
29+
stdout: {stdout}
30+
stderr: {stderr}"
31+
)]
32+
MissingTestResults {
33+
path: PathBuf,
34+
stdout: String,
35+
stderr: String,
36+
},
2737

2838
#[error("File IO error")]
2939
FileError(#[from] FileError),

‎plugins/python3/src/plugin.rs‎

Lines changed: 31 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -181,9 +181,8 @@ impl Python3Plugin {
181181
/// Parse test result file
182182
fn parse_and_verify_test_result(
183183
test_results_json: &Path,
184-
logs: HashMap<String, String>,
185184
hmac_data: Option<(String, String)>,
186-
) -> Result<RunResult, PythonError> {
185+
) -> Result<(RunStatus, Vec<TestResult>), PythonError> {
187186
let results = file_util::read_file_to_string(&test_results_json)?;
188187

189188
// verify test results
@@ -213,7 +212,7 @@ impl Python3Plugin {
213212
.into_iter()
214213
.map(|r| r.into_test_result(&failed_points))
215214
.collect();
216-
Ok(RunResult::new(status, test_results, logs))
215+
Ok((status, test_results))
217216
}
218217
}
219218

@@ -234,9 +233,9 @@ impl LanguagePlugin for Python3Plugin {
234233
file_util::remove_file(&available_points_json)?;
235234
}
236235

237-
let run_result =
238-
Self::run_tmc_command(exercise_directory, &["available_points"], None, None);
239-
if let Err(error) = run_result {
236+
if let Err(error) =
237+
Self::run_tmc_command(exercise_directory, &["available_points"], None, None)
238+
{
240239
log::error!("Failed to scan exercise. {}", error);
241240
}
242241

@@ -280,15 +279,12 @@ impl LanguagePlugin for Python3Plugin {
280279

281280
match output {
282281
Ok(output) => {
283-
let mut logs = HashMap::new();
284-
logs.insert(
285-
"stdout".to_string(),
286-
String::from_utf8_lossy(&output.stdout).into_owned(),
287-
);
288-
logs.insert(
289-
"stderr".to_string(),
290-
String::from_utf8_lossy(&output.stderr).into_owned(),
291-
);
282+
let stdout = String::from_utf8_lossy(&output.stdout);
283+
let stderr = String::from_utf8_lossy(&output.stderr);
284+
log::trace!("stdout: {}", stdout);
285+
log::debug!("stderr: {}", stderr);
286+
287+
// TODO: is it OK to not check output.status.success()?
292288

293289
let hmac_data = if let Some(random_string) = random_string {
294290
let hmac_result_path = exercise_directory.join(".tmc_test_results.hmac.sha256");
@@ -298,27 +294,37 @@ impl LanguagePlugin for Python3Plugin {
298294
None
299295
};
300296

301-
let parse_res =
302-
Self::parse_and_verify_test_result(&test_results_json, logs, hmac_data);
303-
// remove file regardless of parse success
304-
if test_results_json.exists() {
305-
file_util::remove_file(&test_results_json)?;
297+
if !test_results_json.exists() {
298+
return Err(PythonError::MissingTestResults {
299+
path: test_results_json,
300+
stdout: stdout.into_owned(),
301+
stderr: stderr.into_owned(),
302+
}
303+
.into());
306304
}
307-
308-
let mut run_result = parse_res?;
305+
let (status, mut test_results) =
306+
Self::parse_and_verify_test_result(&test_results_json, hmac_data)?;
307+
file_util::remove_file(&test_results_json)?;
309308

310309
// remove points associated with any failed tests
311310
let mut failed_points = HashSet::new();
312-
for test_result in &run_result.test_results {
311+
for test_result in &test_results {
313312
if !test_result.successful {
314313
failed_points.extend(test_result.points.iter().cloned());
315314
}
316315
}
317-
for test_result in &mut run_result.test_results {
316+
for test_result in &mut test_results {
318317
test_result.points.retain(|p| !failed_points.contains(p));
319318
}
320319

321-
Ok(run_result)
320+
let mut logs = HashMap::new();
321+
logs.insert("stdout".to_string(), stdout.into_owned());
322+
logs.insert("stderr".to_string(), stderr.into_owned());
323+
Ok(RunResult {
324+
status,
325+
test_results,
326+
logs,
327+
})
322328
}
323329
Err(PythonError::Tmc(TmcError::Command(CommandError::TimeOut {
324330
stdout,
@@ -899,7 +905,6 @@ class TestClass(unittest.TestCase):
899905
"b379817c66cc7b1610d03ac263f02fa11f7b0153e6aeff3262ecc0598bf0be21".to_string();
900906
Python3Plugin::parse_and_verify_test_result(
901907
temp.path(),
902-
HashMap::new(),
903908
Some((hmac_secret, test_runner_hmac)),
904909
)
905910
.unwrap();
@@ -917,7 +922,6 @@ class TestClass(unittest.TestCase):
917922
"b379817c66cc7b1610d03ac263f02fa11f7b0153e6aeff3262ecc0598bf0be22".to_string();
918923
let res = Python3Plugin::parse_and_verify_test_result(
919924
temp.path(),
920-
HashMap::new(),
921925
Some((hmac_secret, test_runner_hmac)),
922926
);
923927
assert!(res.is_err());

‎plugins/r/src/error.rs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,16 @@ use tmc_langs_util::FileError;
99
pub enum RError {
1010
#[error("Failed to deserialize file {0} into JSON")]
1111
JsonDeserialize(PathBuf, #[source] serde_json::Error),
12+
#[error(
13+
"Failed to locate test results at {path}
14+
stdout: {stdout}
15+
stderr: {stderr}"
16+
)]
17+
MissingTestResults {
18+
path: PathBuf,
19+
stdout: String,
20+
stderr: String,
21+
},
1222

1323
#[error("File IO error")]
1424
FileError(#[from] FileError),

‎plugins/r/src/plugin.rs‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -75,24 +75,37 @@ impl LanguagePlugin for RPlugin {
7575
&["-e", "library(tmcRtestrunner);run_tests()"]
7676
};
7777

78-
if let Some(timeout) = timeout {
79-
let _command = TmcCommand::piped("Rscript")
78+
let output = if let Some(timeout) = timeout {
79+
TmcCommand::piped("Rscript")
8080
.with(|e| e.cwd(path).args(args))
81-
.output_with_timeout_checked(timeout)?;
81+
.output_with_timeout_checked(timeout)?
8282
} else {
83-
let _command = TmcCommand::piped("Rscript")
83+
TmcCommand::piped("Rscript")
8484
.with(|e| e.cwd(path).args(args))
85-
.output_checked()?;
86-
}
85+
.output_checked()?
86+
};
87+
let stdout = String::from_utf8_lossy(&output.stdout);
88+
let stderr = String::from_utf8_lossy(&output.stderr);
89+
log::trace!("stdout: {}", stdout);
90+
log::debug!("stderr: {}", stderr);
8791

8892
// parse test result
93+
if !results_path.exists() {
94+
return Err(RError::MissingTestResults {
95+
path: results_path,
96+
stdout: stdout.into_owned(),
97+
stderr: stderr.into_owned(),
98+
}
99+
.into());
100+
}
89101
let json_file = file_util::open_file(&results_path)?;
90102
let run_result: RRunResult = serde_json::from_reader(json_file).map_err(|e| {
91103
if let Ok(s) = fs::read_to_string(&results_path) {
92-
log::error!("json {}", s);
104+
log::error!("Failed to deserialize json {}", s);
93105
}
94-
RError::JsonDeserialize(results_path, e)
106+
RError::JsonDeserialize(results_path.clone(), e)
95107
})?;
108+
file_util::remove_file(&results_path)?;
96109

97110
Ok(run_result.into())
98111
}

0 commit comments

Comments
 (0)