Implement test helpers for constructing user device data #4476
Conversation
richvdh
force-pushed
the
rav/test_data_rework
branch
5 times, most recently
from
e31eff1 to
25f7927
Compare
| /// Current user's private user-signing key, as an [`Ed25519SecretKey`]. | ||
| pub fn me_private_user_signing_key() -> Ed25519SecretKey { | ||
| Ed25519SecretKey::from_base64(Self::USER_SIGNING_KEY_PRIVATE_EXPORT).unwrap() | ||
| } |
There was a problem hiding this comment.
we end up using this in a few places in a later commit, hence pulling it out to a separate function
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4476 +/- ##
=======================================
Coverage 85.04% 85.05%
=======================================
Files 283 283
Lines 31749 31749
=======================================
+ Hits 27002 27005 +3
+ Misses 4747 4744 -3 ☔ View full report in Codecov by Sentry. |
richvdh
force-pushed
the
rav/test_data_rework
branch
from
a50bd6d to
88b101c
Compare
richvdh
force-pushed
the
rav/test_data_rework
branch
from
88b101c to
1cce2f3
Compare
There was a problem hiding this comment.
I left some comments, I'm not particularly happy with how much code duplication is going on here if we could replace at least a couple of the raw json() calls with types that already define the structure, that would ease my mind.
Thanks for trying to contain the mess our test data produces.
richvdh
force-pushed
the
rav/test_data_rework
branch
from
1cce2f3 to
935dcf1
Compare
We're going to be switching away from JSON-twiddling, so let's snapshot the real object rather than the JSON.
Our test helper won't do this, and it's redundant
... and use it for some simple data
Regenerate Dan's data with new cross-signing and device keys, for which I know the private keys. The signatures are manually calculated for now; this will be improved in a later commit.
richvdh
force-pushed
the
rav/test_data_rework
branch
from
38383db to
f31bd13
Compare
| ); | ||
|
|
||
| let response = builder.build_response(); | ||
| with_settings!({sort_maps => true}, { |
There was a problem hiding this comment.
What happened with the idea of putting the assert_json_snapshot() into the build_response() command?
There was a problem hiding this comment.
Well, I wanted to start with some snapshots before I added build_response, so that I could check what I was changing.
We could probably move the snapshot now, but it's a whole other line of investigation. panic::Location doesn't really work, because it ties the name of the snapshot to the line number where build_response was called, which is obviously not stable. I don't know if we could do something with backtrace, but I'm inclined to leave it for now.
Currently it's very hard to extend our test data because it has been generated manually with real users and devices. I'd like to change that so that the data is created more dynamically, so that it is easier to extend.
This PR introduces a builder type which can be used to help construct such test data, and rebuilds some of the existing test data using it. Unfortunately we're lacking some private keys, so I've had to replace a couple of keys.
The PR also introduces some snapshots, so we can keep an eye on how the data is changing.
Recommend reviewing commit-by-commit.