Skip to content

Commit 0043528

Browse files
fspoetteltguichaoua
andcommitted
fix(review): address review feedback
Co-authored-by: Tristan Guichaoua <33934311+tguichaoua@users.noreply.github.com>
1 parent 28252e2 commit 0043528

File tree

5 files changed

+73
-24
lines changed

5 files changed

+73
-24
lines changed

README.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -116,15 +116,15 @@ cargo all
116116
# Total: 0.20ms
117117
```
118118

119-
This runs all solutions sequentially and prints output to the command-line. Same as for the `solve` command, the `--release` flag runs an optimized build.
119+
This runs all solutions sequentially and prints output to the command-line. Same as for the `solve` command, the `--release` flag runs an optimized build and the `--time` flag outputs benchmarks.
120120

121-
#### Update readme benchmarks
121+
### ➡️ Update readme benchmarks
122122

123-
The template can output a table with solution times to your readme.
123+
The template can write benchmark times to the README via the `cargo time` command.
124124

125-
In order to generate the benchmarking table, run `cargo time`. By default, this command checks for missing benchmarks, runs those solutions, and then updates the table. If you want to (re-)time all solutions, run `cargo time --force` flag. If you want to (re-)time a specific solution, run `cargo time <day>`.
125+
By default, this command checks for missing benchmarks, runs those solutions, and then updates the table. If you want to (re-)time all solutions, run `cargo time --all`. If you want to (re-)time one specific solution, run `cargo time <day>`.
126126

127-
Please note that these are not "scientific" benchmarks, understand them as a fun approximation. 😉 Timings, especially in the microseconds range, might change a bit between invocations.
127+
Please note that these are not _scientific_ benchmarks, understand them as a fun approximation. 😉 Timings, especially in the microseconds range, might change a bit between invocations.
128128

129129
### ➡️ Run all tests
130130

src/main.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ mod args {
3333
time: bool,
3434
},
3535
Time {
36+
all: bool,
3637
day: Option<Day>,
37-
force: bool,
3838
},
3939
#[cfg(feature = "today")]
4040
Today,
@@ -49,10 +49,10 @@ mod args {
4949
time: args.contains("--time"),
5050
},
5151
Some("time") => {
52-
let force = args.contains("--force");
52+
let all = args.contains("--all");
5353

5454
AppArguments::Time {
55-
force,
55+
all,
5656
day: args.opt_free_from_str()?,
5757
}
5858
}
@@ -102,7 +102,7 @@ fn main() {
102102
}
103103
Ok(args) => match args {
104104
AppArguments::All { release, time } => all::handle(release, time),
105-
AppArguments::Time { day, force } => time::handle(day, force),
105+
AppArguments::Time { day, all } => time::handle(day, all),
106106
AppArguments::Download { day } => download::handle(day),
107107
AppArguments::Read { day } => read::handle(day),
108108
AppArguments::Scaffold { day, download } => {

src/template/commands/time.rs

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use crate::template::run_multi::run_multi;
44
use crate::template::timings::Timings;
55
use crate::template::{all_days, readme_benchmarks, Day};
66

7-
pub fn handle(day: Option<Day>, force: bool) {
7+
pub fn handle(day: Option<Day>, recreate_all: bool) {
88
let stored_timings = Timings::read_from_file();
99

1010
let mut days_to_run = HashSet::new();
@@ -15,13 +15,8 @@ pub fn handle(day: Option<Day>, force: bool) {
1515
}
1616
None => {
1717
all_days().for_each(|day| {
18-
// when the force flag is not set, filter out days that are fully benched.
19-
if force
20-
|| !stored_timings
21-
.data
22-
.iter()
23-
.any(|t| t.day == day && t.part_1.is_some() && t.part_2.is_some())
24-
{
18+
// when the `--all` flag is not set, filter out days that are fully benched.
19+
if recreate_all || !stored_timings.is_day_complete(&day) {
2520
days_to_run.insert(day);
2621
}
2722
});

src/template/run_multi.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use super::{
88
};
99

1010
pub fn run_multi(days_to_run: HashSet<Day>, is_release: bool, is_timed: bool) -> Option<Timings> {
11-
let mut timings: Vec<Timing> = vec![];
11+
let mut timings: Vec<Timing> = Vec::with_capacity(days_to_run.len());
1212

1313
all_days().for_each(|day| {
1414
if day > 1 {

src/template/timings.rs

Lines changed: 60 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,8 @@ impl Timings {
2525
/// Dehydrate timings to a JSON file.
2626
pub fn store_file(&self) -> Result<(), Error> {
2727
let json = JsonValue::from(self.clone());
28-
let mut bytes = vec![];
29-
json.format_to(&mut bytes)?;
30-
fs::write(TIMINGS_FILE_PATH, bytes)
28+
let mut file = fs::File::create(TIMINGS_FILE_PATH)?;
29+
json.format_to(&mut file)
3130
}
3231

3332
/// Rehydrate timings from a JSON file. If not present, returns empty timings.
@@ -67,6 +66,12 @@ impl Timings {
6766
pub fn total_millis(&self) -> f64 {
6867
self.data.iter().map(|x| x.total_nanos).sum::<f64>() / 1_000_000_f64
6968
}
69+
70+
pub fn is_day_complete(&self, day: &Day) -> bool {
71+
self.data
72+
.iter()
73+
.any(|t| &t.day == day && t.part_1.is_some() && t.part_2.is_some())
74+
}
7075
}
7176

7277
/* -------------------------------------------------------------------------- */
@@ -101,8 +106,8 @@ impl TryFrom<String> for Timings {
101106
Ok(Timings {
102107
data: json_data
103108
.iter()
104-
.map(|value| Timing::try_from(value).unwrap())
105-
.collect(),
109+
.map(Timing::try_from)
110+
.collect::<Result<_, _>>()?,
106111
})
107112
}
108113
}
@@ -270,7 +275,56 @@ mod tests {
270275
}
271276
}
272277

273-
mod helpers {
278+
mod is_day_complete {
279+
use crate::{
280+
day,
281+
template::timings::{Timing, Timings},
282+
};
283+
284+
#[test]
285+
fn handles_completed_days() {
286+
let timings = Timings {
287+
data: vec![Timing {
288+
day: day!(1),
289+
part_1: Some("1ms".into()),
290+
part_2: Some("2ms".into()),
291+
total_nanos: 3_000_000_000_f64,
292+
}],
293+
};
294+
295+
assert_eq!(timings.is_day_complete(&day!(1)), true);
296+
}
297+
298+
#[test]
299+
fn handles_partial_days() {
300+
let timings = Timings {
301+
data: vec![Timing {
302+
day: day!(1),
303+
part_1: Some("1ms".into()),
304+
part_2: None,
305+
total_nanos: 1_000_000_000_f64,
306+
}],
307+
};
308+
309+
assert_eq!(timings.is_day_complete(&day!(1)), false);
310+
}
311+
312+
#[test]
313+
fn handles_uncompleted_days() {
314+
let timings = Timings {
315+
data: vec![Timing {
316+
day: day!(1),
317+
part_1: None,
318+
part_2: None,
319+
total_nanos: 0.0,
320+
}],
321+
};
322+
323+
assert_eq!(timings.is_day_complete(&day!(1)), false);
324+
}
325+
}
326+
327+
mod merge {
274328
use crate::{
275329
day,
276330
template::timings::{Timing, Timings},

0 commit comments

Comments
 (0)