-
Notifications
You must be signed in to change notification settings - Fork 656
test: custom tests for formatter #2045
Changes from 5 commits
87b7eb1
9068e85
8a47d5d
c622e8d
82d6fd4
293ebbc
e07d418
0a57548
163d149
a362933
5e39ee2
59945e4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,90 @@ | ||
use rome_core::App; | ||
use rome_formatter::{format, FormatOptions}; | ||
use rome_formatter::{format, FormatOptions, IndentStyle}; | ||
use rome_path::RomePath; | ||
use rslint_parser::{parse, Syntax}; | ||
use serde::{Deserialize, Serialize}; | ||
use std::fmt::Debug; | ||
use std::fs; | ||
use std::path::Path; | ||
use std::path::{Path, PathBuf}; | ||
|
||
#[derive(Debug, Eq, PartialEq, Clone, Copy, Deserialize, Serialize)] | ||
pub enum SerializableIndentStyle { | ||
/// Tab | ||
Tab, | ||
/// Space, with its quantity | ||
Space(u8), | ||
} | ||
|
||
impl From<SerializableIndentStyle> for IndentStyle { | ||
fn from(test: SerializableIndentStyle) -> Self { | ||
match test { | ||
SerializableIndentStyle::Tab => IndentStyle::Tab, | ||
SerializableIndentStyle::Space(s) => IndentStyle::Space(s), | ||
} | ||
} | ||
} | ||
|
||
#[derive(Debug, Deserialize, Serialize, Clone)] | ||
pub struct SerializableFormatOptions { | ||
/// The indent style | ||
pub indent_style: Option<SerializableIndentStyle>, | ||
|
||
/// What's the max width of a line. Defaults to 80 | ||
pub line_width: Option<u16>, | ||
} | ||
|
||
impl From<SerializableFormatOptions> for FormatOptions { | ||
fn from(test: SerializableFormatOptions) -> Self { | ||
Self { | ||
indent_style: test | ||
.indent_style | ||
.map_or_else(|| IndentStyle::Tab, |value| value.into()), | ||
line_width: test.line_width.unwrap_or(80), | ||
} | ||
} | ||
} | ||
|
||
#[derive(Debug, Deserialize, Serialize)] | ||
pub struct TestOptions { | ||
cases: Vec<SerializableFormatOptions>, | ||
} | ||
|
||
#[derive(Debug, Default)] | ||
struct SnapshotContent { | ||
input: String, | ||
output: Vec<(String, FormatOptions)>, | ||
} | ||
|
||
impl SnapshotContent { | ||
pub fn add_output(&mut self, content: &str, options: FormatOptions) { | ||
self.output.push((String::from(content), options)) | ||
} | ||
|
||
pub fn set_input(&mut self, content: &str) { | ||
self.input = String::from(content); | ||
} | ||
ematipico marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
pub fn snap_content(&mut self) -> String { | ||
let mut output = String::new(); | ||
output.push_str("# Input"); | ||
output.push('\n'); | ||
output.push_str(self.input.as_str()); | ||
output.push_str("\n=============================\n"); | ||
|
||
output.push_str("# Outputs\n"); | ||
let iter = self.output.iter(); | ||
for (index, (content, options)) in iter.enumerate() { | ||
let formal_index = index + 1; | ||
output.push_str(format!("## Output {formal_index}\n").as_str()); | ||
output.push_str("-----\n"); | ||
output.push_str(format!("{}", options).as_str()); | ||
output.push_str("-----\n"); | ||
output.push_str(content.as_str()); | ||
} | ||
|
||
output | ||
} | ||
} | ||
|
||
/// [insta.rs](https://insta.rs/docs) snapshot testing | ||
/// | ||
|
@@ -22,7 +103,7 @@ use std::path::Path; | |
/// | ||
/// * `json/null` -> input: `tests/specs/json/null.json`, expected output: `tests/specs/json/null.json.snap` | ||
/// * `null` -> input: `tests/specs/null.json`, expected output: `tests/specs/null.json.snap` | ||
pub fn run(spec_input_file: &str, _: &str, file_type: &str) { | ||
pub fn run(spec_input_file: &str, _expected_file: &str, test_directory: &str, file_type: &str) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: The signature starts to become a bit awkward (so many strings). Could be worth having a |
||
let app = App::new(); | ||
let file_path = &spec_input_file; | ||
let spec_input_file = Path::new(spec_input_file); | ||
|
@@ -35,26 +116,47 @@ pub fn run(spec_input_file: &str, _: &str, file_type: &str) { | |
|
||
let mut rome_path = RomePath::new(file_path); | ||
if app.can_format(&rome_path) { | ||
let mut snapshot_content = SnapshotContent::default(); | ||
let buffer = rome_path.get_buffer_from_file(); | ||
let syntax = if file_type == "module" { | ||
Syntax::default().module() | ||
} else { | ||
Syntax::default() | ||
}; | ||
|
||
let input = fs::read_to_string(file_path).unwrap(); | ||
snapshot_content.set_input(input.as_str()); | ||
|
||
let root = parse(buffer.as_str(), 0, syntax).syntax(); | ||
let formatted_result = format(FormatOptions::default(), &root); | ||
let file_name = spec_input_file.file_name().unwrap().to_str().unwrap(); | ||
let input = fs::read_to_string(file_path).unwrap(); | ||
let result = formatted_result.unwrap(); | ||
// we ignore the error for now | ||
let snapshot = format!("# Input\n{}\n---\n# Output\n{}", input, result.as_code()); | ||
let result = formatted_result.unwrap(); | ||
|
||
snapshot_content.add_output(result.as_code(), FormatOptions::default()); | ||
|
||
let test_directory = PathBuf::from(test_directory); | ||
let options_path = test_directory.join("options.json"); | ||
if options_path.exists() { | ||
{ | ||
let mut options_path = RomePath::new(options_path.display().to_string().as_str()); | ||
// SAFETY: we checked its existence already, we assume we have rights to read it | ||
let options: TestOptions = | ||
serde_json::from_str(options_path.get_buffer_from_file().as_str()).unwrap(); | ||
|
||
for test_case in options.cases { | ||
let options = test_case.clone(); | ||
let formatted_result = format(test_case.into(), &root).unwrap(); | ||
snapshot_content.add_output(formatted_result.as_code(), options.into()); | ||
} | ||
} | ||
} | ||
|
||
insta::with_settings!({ | ||
prepend_module_to_snapshot => false, | ||
snapshot_path => spec_input_file.parent().unwrap(), | ||
}, { | ||
insta::assert_snapshot!(file_name, snapshot, file_name); | ||
insta::assert_snapshot!(file_name, snapshot_content.snap_content(), file_name); | ||
}); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,11 +4,11 @@ mod formatter { | |
|
||
mod js_module { | ||
use crate::spec_test; | ||
tests_macros::gen_tests! {"tests/specs/js/module/**/**/*.js", spec_test::run, "module"} | ||
tests_macros::gen_tests! {"tests/specs/js/module/**/**/**/*.js", spec_test::run, "module"} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This shouldn't be needed. |
||
} | ||
|
||
mod js_script { | ||
use crate::spec_test; | ||
tests_macros::gen_tests! {"tests/specs/js/script/**/**/*.js", spec_test::run, "script"} | ||
tests_macros::gen_tests! {"tests/specs/js/script/**/**/**/*.js", spec_test::run, "script"} | ||
} | ||
} |
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "remote derive" feature of serde might be useful for this (the facade types don't have to implement
From
andTestOptions
can deserialize toFormatOptions
directly): https://serde.rs/remote-derive.htmlThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ematipico have you tried this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, and it's not possible with the data structure that I want to have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you seen #2045 (comment)
But agree, this isn't important. I like the name
SerializableFormatOptions
. It makes it clear what it is about.