Background
The GitHub page for the Rust track contains an issue suggesting that the PaaS I/O exercise should have a test case that ensures I/O errors are handled correctly. The issue was created on GitHub in October 2020. A maintainer responded to say they would be willing to review a PR implementing the suggested test cases, but it seems that no PR ever materialized. The last activity on the issue was a comment in May 2021 encouraging a user to try to implement the proposed changes.
I was able to implement tests that would address this gap, and I would be happy to submit a PR if these tests are desired.
Proposed Changes
- Add test cases that require the learner’s implementations
ReadStatsandWriteStatsto propagate errors without logging the failed I/O operation. This would prevent implementations that fail to propagate errors and implementations that increment the read/write count before ensuring that the operation succeeded. For example, the following implementation increments the read count before performing the read, and panics on errors, but it passes the current tests:
use std::io::{Read, Result};
pub struct ReadStats<R> {
reader: R,
bytes_read: usize,
reads: usize,
}
impl<R: Read> Read for ReadStats<R> {
fn read(&mut self, buf: &mut [u8]) -> Result<usize> {
self.reads += 1;
let bytes_read = self.reader.read(buf).unwrap();
self.bytes_read += bytes_read;
Ok(bytes_read)
}
}
-
Possibly update the problem description to clarify what
ReadStatsandWriteStatsare supposed to do with errors. -
If it is determined that the first two changes should not be made, then the GitHub issue suggesting them should be closed so that people like me don’t waste their time in the future.
Impact
Adding these tests would require re-testing past submissions. It may very well not be worth the cost to do so, but below are some reasons that it might be. Again, if the maintainers decide not to move forward with adding these tests, then the GitHub issue should be closed.
These tests would help learners to practice error handling without complicating the exercise much, if at all (in my opinion, it’s simpler to propagate the error than it is to unwrap it). Although I understand that the goal of Exercism is not to require learners to write production-quality code for every exercise, I think that this exercise is particularly well-suited to being used to reinforce good error handling habits. Not only is it rather simple to propagate I/O errors in this exercise, but it also fits well with the “backstory” of the problem description. If you’re making a PaaS, you probably don’t want to charge customers for failed operations, so you want to make sure to only increase the read/write count after the operation succeeds.
The new tests would invalidate many previously accepted solutions. However, those solutions, like the code snippet above, are incorrect. In the code snippet above, we are not reporting “the total number of read/write operations” as the problem description demands. We are reporting the total number of attempted read/write operations.
The tests for this exercise are already different for different tracks, so adding these test cases to the Rust track would do little to worsen that situation. However, updating the problem description to explain how errors should be handled would fragment the exercise’s implementations across different tracks. As such, it may be better to just add the tests and expect learners to deduce what they are meant to do from the error message they get when the tests fail.
