Skip to content

Commit 44d930c

Browse files
noahbaldnoahbald
committed
chore: Improve tests and fix issues
1 parent c8b48bd commit 44d930c

File tree

34 files changed

+1363
-282
lines changed

34 files changed

+1363
-282
lines changed

‎Cargo.lock

+40
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Cargo.toml

+2
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,15 @@ oxvg_optimiser = { path = "crates/oxvg_optimiser" }
1919
oxvg_selectors = { path = "crates/oxvg_selectors" }
2020
oxvg_utils = { path = "crates/oxvg_utils" }
2121

22+
anyhow = "1.0"
2223
# NOTE: Out of date version used for compatibility with selectors
2324
# https://github.com/servo/stylo/blob/main/Cargo.toml#L35
2425
cssparser = "0.31.0"
2526
derivative = "2.2"
2627
insta = "1.36.1"
2728
markup5ever = "0.12"
2829
rcdom = { package = "markup5ever_rcdom", version = "0.3" }
30+
regex = "1.10"
2931
serde = "1.0.197"
3032
serde_json = "1.0.114"
3133
quick-xml = "0.31.0"

‎crates/oxvg/Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ test = false
1717
[dependencies]
1818
oxvg_optimiser = { workspace = true }
1919

20-
anyhow = "1.0"
20+
anyhow = { workspace = true }
2121
clap = { version = "4.5.4", features = ["derive"] }
2222
config = { version = "0.14.0", features = ["json", "json5"] }
2323
rcdom = { workspace = true }

‎crates/oxvg/src/config.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use serde::Deserialize;
22

33
#[derive(Deserialize, Default)]
4+
#[serde(rename_all = "camelCase")]
45
pub struct Config {
56
pub optimisation: Option<oxvg_optimiser::Jobs>,
67
}

‎crates/oxvg_ast/src/node.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,8 @@ impl<'de> Visitor<'de> for AttributesVisitor {
9999
{
100100
let mut attributes = Attributes::default();
101101

102-
while let Some((key, value)) = map.next_entry::<LocalName, String>()? {
103-
let value = value.into();
102+
while let Some((key, value)) = map.next_entry::<LocalName, Option<String>>()? {
103+
let value = value.unwrap_or_else(String::new).into();
104104
attributes.insert(key, value);
105105
}
106106

‎crates/oxvg_optimiser/Cargo.toml

+2
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,13 @@ serde_json = { workspace = true }
1919
markup5ever = { workspace = true }
2020
miette = { version = "7.0.0", features = ["fancy"] }
2121
rcdom = { workspace = true }
22+
regex = { workspace = true }
2223
thiserror = "1.0.56"
2324
urlencoding = "2.1"
2425

2526
[dev-dependencies]
2627
oxvg_ast = { workspace = true }
2728

29+
anyhow = { workspace = true }
2830
insta = { workspace = true }
2931
xml5ever = { workspace = true }

‎crates/oxvg_optimiser/src/jobs/add_attributes_to_svg_element.rs

+44-41
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
use std::{collections::HashSet, rc::Rc};
22

3+
use markup5ever::local_name;
34
use oxvg_ast::Attributes;
45
use serde::Deserialize;
56

67
use crate::Job;
78

89
#[derive(Deserialize, Default, Clone)]
10+
#[serde(rename_all = "camelCase")]
911
pub struct AddAttributesToSVGElement {
1012
pub attributes: Attributes,
1113
}
@@ -14,7 +16,12 @@ impl Job for AddAttributesToSVGElement {
1416
fn run(&self, node: &Rc<rcdom::Node>) {
1517
use rcdom::NodeData::Element;
1618

17-
let Element { attrs, .. } = &node.data else {
19+
let Element { attrs, name, .. } = &node.data else {
20+
return;
21+
};
22+
23+
let is_root = oxvg_selectors::Element::new(node.clone()).is_root();
24+
if name.local != local_name!("svg") || !is_root {
1825
return;
1926
};
2027
let attrs = &mut *attrs.borrow_mut();
@@ -31,49 +38,45 @@ impl Job for AddAttributesToSVGElement {
3138
}
3239

3340
#[test]
34-
fn add_attributes_to_svg_element() -> Result<(), &'static str> {
35-
use rcdom::NodeData::Element;
36-
use xml5ever::{
37-
driver::{parse_document, XmlParseOpts},
38-
tendril::TendrilSink,
39-
};
41+
fn add_attributes_to_svg_element() -> anyhow::Result<()> {
42+
use crate::test_config;
4043

41-
let dom: rcdom::RcDom =
42-
parse_document(rcdom::RcDom::default(), XmlParseOpts::default()).one("<svg></svg>");
43-
let root = &dom.document.children.borrow()[0];
44-
let job = &mut AddAttributesToSVGElement::default();
44+
insta::assert_snapshot!(test_config(
45+
// Add multiple attributes without value
46+
r#"{ "addAttributesToSvgElement": {
47+
"attributes": { "data-icon": null, "className={classes}": null }
48+
} }"#,
49+
None,
50+
)?);
4551

46-
job.attributes
47-
.insert(markup5ever::LocalName::from("foo"), "bar".into());
48-
job.run(root);
49-
match &root.data {
50-
Element { attrs, .. } => {
51-
assert_eq!(
52-
attrs.borrow().last(),
53-
Some(&markup5ever::Attribute {
54-
name: markup5ever::QualName::new(None, ns!(svg), "foo".into()),
55-
value: "bar".into()
56-
})
57-
);
58-
}
59-
_ => Err("Attribute not added")?,
60-
}
52+
insta::assert_snapshot!(test_config(
53+
// Add single attribute without value
54+
r#"{ "addAttributesToSvgElement": {
55+
"attributes": { "data-icon": null }
56+
} }"#,
57+
None,
58+
)?);
6159

62-
job.attributes
63-
.insert(markup5ever::LocalName::from("foo"), "baz".into());
64-
job.run(root);
65-
match &root.data {
66-
Element { attrs, .. } => {
67-
assert_eq!(
68-
attrs.borrow().last(),
69-
Some(&markup5ever::Attribute {
70-
name: markup5ever::QualName::new(None, ns!(svg), "foo".into()),
71-
value: "bar".into()
72-
})
73-
);
74-
}
75-
_ => Err("Attribute not added")?,
76-
}
60+
insta::assert_snapshot!(test_config(
61+
// Add multiple attributes with values
62+
r#"{ "addAttributesToSvgElement": {
63+
"attributes": { "focusable": "false", "data-image": "icon" }
64+
} }"#,
65+
None,
66+
)?);
67+
68+
insta::assert_snapshot!(test_config(
69+
// Ignore nested <svg> elements
70+
r#"{ "addAttributesToSvgElement": {
71+
"attributes": { "data-icon": null }
72+
} }"#,
73+
Some(
74+
r#"<svg xmlns="http://www.w3.org/2000/svg">
75+
test
76+
<svg />
77+
</svg>"#
78+
),
79+
)?);
7780

7881
Ok(())
7982
}

‎crates/oxvg_optimiser/src/jobs/add_classes_to_svg.rs

+24-26
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use serde::Deserialize;
77
use crate::Job;
88

99
#[derive(Deserialize, Default, Clone)]
10+
#[serde(rename_all = "camelCase")]
1011
pub struct AddClassesToSVG {
1112
pub class_names: Option<Vec<String>>,
1213
pub class_name: Option<String>,
@@ -52,34 +53,31 @@ impl Job for AddClassesToSVG {
5253
}
5354

5455
#[test]
55-
fn add_classes_to_svg() -> Result<(), &'static str> {
56-
use rcdom::NodeData::Element;
57-
use xml5ever::{
58-
driver::{parse_document, XmlParseOpts},
59-
tendril::TendrilSink,
60-
};
56+
fn add_classes_to_svg() -> anyhow::Result<()> {
57+
use crate::test_config;
6158

62-
let dom: rcdom::RcDom =
63-
parse_document(rcdom::RcDom::default(), XmlParseOpts::default()).one("<svg></svg>");
64-
let root = &dom.document.children.borrow()[0];
65-
let job = AddClassesToSVG {
66-
class_names: Some(vec![String::from("foo"), String::from("bar")]),
67-
class_name: None,
68-
};
59+
insta::assert_snapshot!(test_config(
60+
// Should add classes when passed as a classNames Array
61+
r#"{ "addClassesToSvg": {
62+
"classNames": ["mySvg", "size-big"],
63+
} }"#,
64+
None
65+
)?);
6966

70-
job.run(root);
71-
let attrs = match &root.data {
72-
Element { attrs, .. } => attrs,
73-
_ => Err("Unexpected document structure")?,
74-
};
75-
let attrs = &*attrs.borrow();
76-
let Some(class) = attrs
77-
.iter()
78-
.find(|attr| attr.name.local.to_string() == "class")
79-
else {
80-
unreachable!("Class attribute missing");
81-
};
82-
assert_eq!(class.value, "foo bar".into());
67+
insta::assert_snapshot!(test_config(
68+
// Should add class when passed as a className String
69+
r#"{ "addClassesToSvg": {
70+
"className": "mySvg",
71+
} }"#,
72+
None
73+
)?);
8374

75+
insta::assert_snapshot!(test_config(
76+
// Should avoid adding existing classes
77+
r#"{ "addClassesToSvg": {
78+
"classNames": "mySvg size-big",
79+
} }"#,
80+
Some(r#"<svg xmlns="http://www.w3.org/2000/svg" class="mySvg">"#)
81+
)?);
8482
Ok(())
8583
}

‎crates/oxvg_optimiser/src/jobs/cleanup_attributes.rs

+30-26
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use serde::Deserialize;
55
use crate::Job;
66

77
#[derive(Deserialize, Default, Clone)]
8+
#[serde(rename_all = "camelCase")]
89
pub struct CleanupAttributes {
910
newlines: Option<bool>,
1011
trim: Option<bool>,
@@ -40,34 +41,37 @@ impl Job for CleanupAttributes {
4041
}
4142

4243
#[test]
43-
fn cleanup_attributes() -> Result<(), &'static str> {
44-
use rcdom::NodeData::Element;
45-
use xml5ever::{
46-
driver::{parse_document, XmlParseOpts},
47-
tendril::TendrilSink,
48-
};
44+
fn cleanup_attributes() -> anyhow::Result<()> {
45+
use crate::test_config;
4946

50-
let dom: rcdom::RcDom = parse_document(rcdom::RcDom::default(), XmlParseOpts::default()).one(
51-
r#"<svg class=" foo bar
52-
baz"></svg>"#,
53-
);
54-
let root = &dom.document.children.borrow()[0];
55-
let job = CleanupAttributes {
56-
newlines: Some(true),
57-
trim: Some(true),
58-
spaces: Some(true),
59-
};
47+
insta::assert_snapshot!(test_config(
48+
r#"{ "cleanupAttributes": {
49+
"newlines": true,
50+
"trim": true,
51+
spaces: true
52+
} }"#,
53+
Some(
54+
r#"<svg xmlns=" http://www.w3.org/2000/svg
55+
" attr="a b" attr2="a
56+
b">
57+
test
58+
</svg>"#
59+
)
60+
)?);
6061

61-
job.run(root);
62-
let attrs = match &root.data {
63-
Element { attrs, .. } => attrs,
64-
_ => Err("Unexpected document structure")?,
65-
};
66-
let attrs = &*attrs.borrow();
67-
assert_eq!(
68-
attrs.first().map(|attr| &attr.value),
69-
Some(&"foo bar baz".into())
70-
);
62+
insta::assert_snapshot!(test_config(
63+
r#"{ "cleanupAttributes": {
64+
"newlines": true,
65+
"trim": true,
66+
spaces: true
67+
} }"#,
68+
Some(
69+
r#"<svg xmlns=" http://www.w3.org/2000/svg
70+
" attr="a b">
71+
test &amp; &lt;&amp; &gt; &apos; &quot; &amp;
72+
</svg>"#
73+
)
74+
)?);
7175

7276
Ok(())
7377
}

0 commit comments

Comments
 (0)