Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(snapshot): Use snapshot testing in sdk-common #4428

Merged
merged 1 commit into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ concurrency:

env:
CARGO_TERM_COLOR: always
# Insta.rs is run directly via cargo test. We don't want insta.rs to create new snapshots files.
# Just want it to run the tests (option `no` instead of `auto`).
INSTA_UPDATE: no

jobs:
xtask:
Expand Down
27 changes: 27 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,33 @@ integration tests that need a running synapse instance. These tests reside in
synapse for testing purposes.


### Snapshot Testing

You can add/review snapshot tests using [insta.rs](https://insta.rs)

Every new struct/enum that derives `Serialize` `Deserialise` should have a snapshot test for it.
Any code change that breaks serialisation will then break a test, the author will then have to decide
how to handle migration and test it if needed.


And for an improved review experience it's recommended (but not necessary) to install the cargo-insta tool:

Unix:
```
curl -LsSf https://insta.rs/install.sh | sh
```

Windows:
```
powershell -c "irm https://insta.rs/install.ps1 | iex"
```

Usual flow is to first run the test, then review them.
```
cargo insta test
cargo insta review
```

## Pull requests

Ideally, a PR should have a *proper title*, with *atomic logical commits*, and
Expand Down
20 changes: 20 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ hmac = "0.12.1"
http = "1.1.0"
imbl = "3.0.0"
indexmap = "2.6.0"
insta = { version = "1.41.1", features = ["json"] }
itertools = "0.13.0"
js-sys = "0.3.69"
mime = "0.3.17"
Expand Down Expand Up @@ -124,6 +125,9 @@ debug = 0
# for the extra time of optimizing it for a clean build of matrix-sdk-ffi.
quote = { opt-level = 2 }
sha2 = { opt-level = 2 }
# faster runs for insta.rs snapshot testing
BillCarsonFr marked this conversation as resolved.
Show resolved Hide resolved
insta.opt-level = 3
similar.opt-level = 3

# Custom profile with full debugging info, use `--profile dbg` to select
[profile.dbg]
Expand Down
1 change: 1 addition & 0 deletions crates/matrix-sdk-common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ assert_matches = { workspace = true }
proptest = { workspace = true }
matrix-sdk-test-macros = { path = "../../testing/matrix-sdk-test-macros" }
wasm-bindgen-test = { workspace = true }
insta = { workspace = true }

[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
# Enable the test macro.
Expand Down
138 changes: 132 additions & 6 deletions crates/matrix-sdk-common/src/deserialized_responses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -909,21 +909,22 @@ mod tests {
use std::collections::BTreeMap;

use assert_matches::assert_matches;
use insta::{assert_json_snapshot, with_settings};
use ruma::{
event_id,
device_id, event_id,
events::{room::message::RoomMessageEventContent, AnySyncTimelineEvent},
serde::Raw,
user_id,
user_id, DeviceKeyAlgorithm,
};
use serde::Deserialize;
use serde_json::json;

use super::{
AlgorithmInfo, DecryptedRoomEvent, EncryptionInfo, SyncTimelineEvent, TimelineEvent,
TimelineEventKind, UnableToDecryptInfo, UnableToDecryptReason, UnsignedDecryptionResult,
UnsignedEventLocation, VerificationState, WithheldCode,
AlgorithmInfo, DecryptedRoomEvent, DeviceLinkProblem, EncryptionInfo, ShieldState,
ShieldStateCode, SyncTimelineEvent, TimelineEvent, TimelineEventKind, UnableToDecryptInfo,
UnableToDecryptReason, UnsignedDecryptionResult, UnsignedEventLocation, VerificationLevel,
VerificationState, WithheldCode,
};
use crate::deserialized_responses::{DeviceLinkProblem, ShieldStateCode, VerificationLevel};

fn example_event() -> serde_json::Value {
json!({
Expand Down Expand Up @@ -1317,4 +1318,129 @@ mod tests {
let reason = UnableToDecryptReason::UnknownMegolmMessageIndex;
assert!(reason.is_missing_room_key());
}

#[test]
fn snapshot_test_verification_level() {
assert_json_snapshot!(VerificationLevel::VerificationViolation);
assert_json_snapshot!(VerificationLevel::UnsignedDevice);
assert_json_snapshot!(VerificationLevel::None(DeviceLinkProblem::InsecureSource));
assert_json_snapshot!(VerificationLevel::None(DeviceLinkProblem::MissingDevice));
assert_json_snapshot!(VerificationLevel::UnverifiedIdentity);
}

#[test]
fn snapshot_test_verification_states() {
assert_json_snapshot!(VerificationState::Unverified(VerificationLevel::UnsignedDevice));
assert_json_snapshot!(VerificationState::Unverified(
VerificationLevel::VerificationViolation
));
assert_json_snapshot!(VerificationState::Unverified(VerificationLevel::None(
DeviceLinkProblem::InsecureSource,
)));
assert_json_snapshot!(VerificationState::Unverified(VerificationLevel::None(
DeviceLinkProblem::MissingDevice,
)));
assert_json_snapshot!(VerificationState::Verified);
}

#[test]
fn snapshot_test_shield_states() {
assert_json_snapshot!(ShieldState::None);
assert_json_snapshot!(ShieldState::Red {
code: ShieldStateCode::UnverifiedIdentity,
message: "a message"
});
assert_json_snapshot!(ShieldState::Grey {
code: ShieldStateCode::AuthenticityNotGuaranteed,
message: "authenticity of this message cannot be guaranteed",
});
}

#[test]
fn snapshot_test_shield_codes() {
assert_json_snapshot!(ShieldStateCode::AuthenticityNotGuaranteed);
assert_json_snapshot!(ShieldStateCode::UnknownDevice);
assert_json_snapshot!(ShieldStateCode::UnsignedDevice);
assert_json_snapshot!(ShieldStateCode::UnverifiedIdentity);
assert_json_snapshot!(ShieldStateCode::SentInClear);
assert_json_snapshot!(ShieldStateCode::VerificationViolation);
}

#[test]
fn snapshot_test_algorithm_info() {
let mut map = BTreeMap::new();
map.insert(DeviceKeyAlgorithm::Curve25519, "claimedclaimedcurve25519".to_owned());
map.insert(DeviceKeyAlgorithm::Ed25519, "claimedclaimeded25519".to_owned());
let info = AlgorithmInfo::MegolmV1AesSha2 {
curve25519_key: "curvecurvecurve".into(),
sender_claimed_keys: BTreeMap::from([
(DeviceKeyAlgorithm::Curve25519, "claimedclaimedcurve25519".to_owned()),
(DeviceKeyAlgorithm::Ed25519, "claimedclaimeded25519".to_owned()),
]),
};

assert_json_snapshot!(info)
}

#[test]
fn snapshot_test_encryption_info() {
let info = EncryptionInfo {
sender: user_id!("@alice:localhost").to_owned(),
sender_device: Some(device_id!("ABCDEFGH").to_owned()),
algorithm_info: AlgorithmInfo::MegolmV1AesSha2 {
curve25519_key: "curvecurvecurve".into(),
sender_claimed_keys: Default::default(),
},
verification_state: VerificationState::Verified,
};

with_settings!({sort_maps =>true}, {
assert_json_snapshot!(info)
})
}

#[test]
fn snapshot_test_sync_timeline_event() {
let room_event = SyncTimelineEvent {
kind: TimelineEventKind::Decrypted(DecryptedRoomEvent {
event: Raw::new(&example_event()).unwrap().cast(),
encryption_info: EncryptionInfo {
sender: user_id!("@sender:example.com").to_owned(),
sender_device: Some(device_id!("ABCDEFGHIJ").to_owned()),
algorithm_info: AlgorithmInfo::MegolmV1AesSha2 {
curve25519_key: "xxx".to_owned(),
sender_claimed_keys: BTreeMap::from([
(
DeviceKeyAlgorithm::Ed25519,
"I3YsPwqMZQXHkSQbjFNEs7b529uac2xBpI83eN3LUXo".to_owned(),
),
(
DeviceKeyAlgorithm::Curve25519,
"qzdW3F5IMPFl0HQgz5w/L5Oi/npKUFn8Um84acIHfPY".to_owned(),
),
]),
},
verification_state: VerificationState::Verified,
},
unsigned_encryption_info: Some(BTreeMap::from([(
UnsignedEventLocation::RelationsThreadLatestEvent,
UnsignedDecryptionResult::UnableToDecrypt(UnableToDecryptInfo {
session_id: Some("xyz".to_owned()),
reason: UnableToDecryptReason::MissingMegolmSession {
withheld_code: Some(WithheldCode::Unverified),
},
}),
)])),
}),
push_actions: Default::default(),
};

with_settings!({sort_maps =>true}, {
// We use directly the serde_json formatter here, because of a bug in insta
// not serializing custom BTreeMap key enum https://github.com/mitsuhiko/insta/issues/689
assert_json_snapshot! {
serde_json::to_value(&room_event).unwrap(),
}
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
source: crates/matrix-sdk-common/src/deserialized_responses.rs
expression: info
---
{
"MegolmV1AesSha2": {
BillCarsonFr marked this conversation as resolved.
Show resolved Hide resolved
"curve25519_key": "curvecurvecurve",
"sender_claimed_keys": {
"ed25519": "claimedclaimeded25519",
"curve25519": "claimedclaimedcurve25519"
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
source: crates/matrix-sdk-common/src/deserialized_responses.rs
expression: info
---
{
"sender": "@alice:localhost",
"sender_device": "ABCDEFGH",
"algorithm_info": {
"MegolmV1AesSha2": {
"curve25519_key": "curvecurvecurve",
"sender_claimed_keys": {}
}
},
"verification_state": "Verified"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: crates/matrix-sdk-common/src/deserialized_responses.rs
expression: "serde_json::to_value(&code).unwrap()"
---
"UnknownDevice"
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: crates/matrix-sdk-common/src/deserialized_responses.rs
expression: "serde_json::to_value(&code).unwrap()"
---
"UnsignedDevice"
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: crates/matrix-sdk-common/src/deserialized_responses.rs
expression: "serde_json::to_value(&code).unwrap()"
---
"UnverifiedIdentity"
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: crates/matrix-sdk-common/src/deserialized_responses.rs
expression: "serde_json::to_value(&code).unwrap()"
---
"SentInClear"
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: crates/matrix-sdk-common/src/deserialized_responses.rs
expression: "serde_json::to_value(&code).unwrap()"
---
"VerificationViolation"
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: crates/matrix-sdk-common/src/deserialized_responses.rs
expression: "serde_json::to_value(&code).unwrap()"
---
"AuthenticityNotGuaranteed"
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
source: crates/matrix-sdk-common/src/deserialized_responses.rs
expression: "serde_json::to_value(&state).unwrap()"
---
{
"Red": {
"code": "UnverifiedIdentity",
"message": "a message"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
source: crates/matrix-sdk-common/src/deserialized_responses.rs
expression: "serde_json::to_value(&state).unwrap()"
---
{
"Grey": {
"code": "AuthenticityNotGuaranteed",
"message": "authenticity of this message cannot be guaranteed"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: crates/matrix-sdk-common/src/deserialized_responses.rs
expression: "serde_json::to_value(&state).unwrap()"
---
"None"
Loading
Loading