Puts create_tmp_accounts_dir_for_tests() behind DCOU #34022
Conversation
brooksprumo
force-pushed
the
snap/dcou/create_tmp_accounts_dir_for_tests
branch
from
ec8b9e9 to
e084ec9
Compare
brooksprumo
added
the
automerge
Codecov Report
@@ Coverage Diff @@
## master #34022 +/- ##
=========================================
- Coverage 81.9% 81.9% -0.1%
=========================================
Files 811 811
Lines 219427 219429 +2
=========================================
- Hits 179813 179754 -59
- Misses 39614 39675 +61 |
| @@ -1999,7 +1997,9 @@ pub fn verify_snapshot_archive( | |||
| ) { | |||
| let temp_dir = tempfile::TempDir::new().unwrap(); | |||
| let unpack_dir = temp_dir.path(); | |||
| let unpack_account_dir = create_accounts_run_and_snapshot_dirs(unpack_dir).unwrap().0; | |||
| let unpack_account_dir = accounts_db::create_accounts_run_and_snapshot_dirs(unpack_dir) | |||
There was a problem hiding this comment.
nit: I've found the tuple indexing to be kind of tricky sometimes. Doing the full de-structure is a bit more explicit:
let (unpack_account_dir, _snapshot_dir) = ...This isn't perfect either as someone could still swap the order of the return types without breaking anything. I guess a struct with names fields, and unpacking by named field would be the only way to fully protect against this.
In any case, feel free to change or ignore; certainly don't want to punish the good deed of restricting access to the test only function
There was a problem hiding this comment.
Lol, missed that you added auto-merge label. Like I said, I was fine pushing as-is anyways
There was a problem hiding this comment.
Yeah, tuples aren't the most explicit. I agree with you, I think destructing is probably the better call.
Problem
create_tmp_accounts_dir_for_tests()is used by tests to create a temp "accounts path" directory. But nothing is stopping a non-test from calling it...Summary of Changes
...until now! Put it behind DCOU.
(and give the function some documentation too, for good measure)