test: refactor formatter testing infra#9588
Merged
Conversation
|
Merging this PR will not alter performance
Comparing Footnotes
|
Contributor
WalkthroughThis PR introduces Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/biome_html_formatter/tests/quick_test.rs (1)
12-20:⚠️ Potential issue | 🟡 MinorOpen this sample as Svelte, not plain HTML.
{#snippet...}is Svelte syntax, buttest.htmlwithdocument_file_source: Nonewill be treated as HTML. If someone unignores this, they’ll be debugging the wrong parser.🛠️ One simple fix
- let path = BiomePath::new("test.html"); + let path = BiomePath::new("test.svelte");Based on learnings: Inspect AST structure before implementing parser/formatter changes using the parser crate's
quick_test.Also applies to: 31-43
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_html_formatter/tests/quick_test.rs` around lines 12 - 20, The sample test input (variable src) contains Svelte-specific syntax (`{`#snippet` ...}`) but is being run as plain HTML; update the test invocation that calls the parser/quick_test helper (in this file's test function using quick_test from the parser crate) to provide a Svelte document source/extension so the parser treats it as Svelte (for example set document_file_source to Some("test.svelte") or otherwise pass a filename with .svelte) before running assertions; locate the call that consumes src in quick_test.rs and change its test harness/argument to indicate Svelte so the Svelte AST is produced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_formatter_test/src/spec.rs`:
- Around line 309-312: The test currently skips assertions when re-formatting
fails or produces empty output, masking formatter regressions; in the block
handling the formatter result (the match that yields Err(_) and the subsequent
check using re_printed.as_code()), change the Err(_) => return early-return into
a test failure or logged panic that includes the error details from the
formatter, and remove (or at least replace with an explicit assertion) the
!re_printed.as_code().is_empty() guard so that empty formatted output causes the
test to fail and surface the issue (use the existing re_printed and its
as_code() for the message).
In `@crates/biome_html_formatter/tests/spec_test.rs`:
- Around line 10-12: The test currently panics when SpecTestFile::try_from_file
returns None (e.g., for options.json), so change the panic into a graceful skip:
replace the `let Some(test_file) = SpecTestFile::try_from_file(...) else {
panic!(...) }` pattern with an explicit match or if-let that returns early or
continues the test harness when None is returned (e.g., `if let Some(test_file)
= SpecTestFile::try_from_file(...) { /* run assertions */ } else { return; }`),
ensuring the harness skips spec directories that intentionally produce None
instead of failing; keep all processing logic that uses the resulting test_file
(the test_file variable / SpecTestFile) unchanged.
---
Outside diff comments:
In `@crates/biome_html_formatter/tests/quick_test.rs`:
- Around line 12-20: The sample test input (variable src) contains
Svelte-specific syntax (`{`#snippet` ...}`) but is being run as plain HTML; update
the test invocation that calls the parser/quick_test helper (in this file's test
function using quick_test from the parser crate) to provide a Svelte document
source/extension so the parser treats it as Svelte (for example set
document_file_source to Some("test.svelte") or otherwise pass a filename with
.svelte) before running assertions; locate the call that consumes src in
quick_test.rs and change its test harness/argument to indicate Svelte so the
Svelte AST is produced.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7ad2119e-c644-4758-bbae-961e1e35827f
⛔ Files ignored due to path filters (282)
Cargo.lockis excluded by!**/*.lockand included by**crates/biome_css_formatter/tests/specs/css/atrule/charset.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/atrule/color_profile.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/atrule/container.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/atrule/counter_style.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/atrule/document.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/atrule/font_face.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/atrule/font_feature_values.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/atrule/function.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/atrule/import.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/atrule/keyframes.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/atrule/layer.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/atrule/media.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/atrule/namespace.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/atrule/page.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/atrule/page_complex.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/atrule/property.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/atrule/scope.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/atrule/starting_style.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/atrule/supports.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/atrule/supports_complex.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/atrule/tailwind/source-not.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/atrule/unknown.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/atrule/value.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/attr.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/block.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/casing.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/color/functional_colors.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/color/hex_colors.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/comma_separated_values.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/composes.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/declaration_empty.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/declaration_list.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/dimensions.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/empty.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/functions.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/global_suppression.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/if.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/important.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/issue_3229.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/issue_4298.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/issue_8636.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/namespace.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/nesting/conditional_at_rule.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/nesting/nesting.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/nesting/nesting_1.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/number.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/properties/all.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/properties/border.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/properties/custom.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/properties/generic.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/properties/grid.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/properties/unicode_range.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/properties/z-index.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/pseudo/is.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/pseudo/not.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/pseudo/pseudo_class_function_compound_selector.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/pseudo/pseudo_class_function_compound_selector_list.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/pseudo/pseudo_class_function_custom_identifier_list.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/pseudo/pseudo_class_function_nth.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/pseudo/pseudo_class_function_relative_selector_list.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/pseudo/pseudo_class_function_selector.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/pseudo/pseudo_class_function_state.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/pseudo/pseudo_class_function_value_list.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/pseudo/pseudo_class_identifier.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/pseudo/pseudo_element_selector.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/pseudo/where.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/quote_style/normalize_quotes.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/range/between_rules.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/range/keyframes.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/range/mid_value.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/range/selector_list.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/range/single_declaration.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/range/single_rule.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/range/single_value.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/ratio/issue_9249.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/at-rule/at-root.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/at-rule/container-general-enclosed.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/at-rule/container-scroll-state.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/at-rule/content.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/at-rule/debug-warn-error.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/at-rule/declaration-block-interpolation.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/at-rule/each.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/at-rule/else.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/at-rule/extend.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/at-rule/for.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/at-rule/forward.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/at-rule/function.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/at-rule/if-else.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/at-rule/import.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/at-rule/include.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/at-rule/mixin.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/at-rule/return.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/at-rule/supports-expression.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/at-rule/supports-general-enclosed.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/at-rule/use.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/at-rule/while.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/declaration/ambiguous-selector-vs-nesting.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/declaration/bracketed-values.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/declaration/global-flag.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/declaration/interpolation.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/declaration/mixed.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/declaration/namespaced.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/declaration/nested-properties-complex-value.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/declaration/nested-properties-empty-value.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/declaration/nested-properties-with-value.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/declaration/page-at-rule.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/declaration/parent-and-colon-values.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/declaration/simple-variable.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/declaration/spacing-after-colon.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/expression/bracketed-values.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/expression/edge-cases.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/expression/formatting.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/expression/function-call.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/expression/interpolation.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/expression/list-map-paren.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/expression/precedence.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/expression/qualified-names.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/expression/signed-numeric-operator.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/selector/interpolation.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/selectors/attribute_selector.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/selectors/class_selector.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/selectors/complex_selector.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/selectors/id_selector.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/selectors/pseudo_class/pseudo_class_has.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/selectors/selector_lists.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/selectors/type_selector.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/simple.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/tailwind/long-apply.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/tailwind/plugin-no-options.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/tailwind/plugin-with-options.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/tailwind/shadcn-default.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/tailwind/source-inline.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/tailwind/supports.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/tailwind/utility.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/trailing_newline/simple.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/unary-precedence.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/units.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/url.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/value_fill.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/value_one_group_per_line.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/value_one_per_line.css.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/variables.css.snapis excluded by!**/*.snapand included by**crates/biome_graphql_formatter/tests/specs/graphql/definitions/directive_definition.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_formatter/tests/specs/graphql/definitions/enum.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_formatter/tests/specs/graphql/definitions/enum_extension.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_formatter/tests/specs/graphql/definitions/fragment.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_formatter/tests/specs/graphql/definitions/input_object.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_formatter/tests/specs/graphql/definitions/input_object_extension.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_formatter/tests/specs/graphql/definitions/interface.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_formatter/tests/specs/graphql/definitions/interface_extension.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_formatter/tests/specs/graphql/definitions/object.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_formatter/tests/specs/graphql/definitions/object_extension.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_formatter/tests/specs/graphql/definitions/scalar.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_formatter/tests/specs/graphql/definitions/scalar_extension.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_formatter/tests/specs/graphql/definitions/schema.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_formatter/tests/specs/graphql/definitions/schema_extension.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_formatter/tests/specs/graphql/definitions/union.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_formatter/tests/specs/graphql/definitions/union_extension.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_formatter/tests/specs/graphql/directive.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_formatter/tests/specs/graphql/newline_clrf.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_formatter/tests/specs/graphql/object/spacing/global/object_spacing.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_formatter/tests/specs/graphql/object/spacing/object_spacing.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_formatter/tests/specs/graphql/operation.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_formatter/tests/specs/graphql/selection_set.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_formatter/tests/specs/graphql/simple.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_formatter/tests/specs/graphql/suppression.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_formatter/tests/specs/graphql/trailing_newline/simple.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_formatter/tests/specs/graphql/type.graphql.snapis excluded by!**/*.snapand included by**crates/biome_graphql_formatter/tests/specs/graphql/value.graphql.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/bubble.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/comments/multifile.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/comments/top-comments.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/file_node.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/function_definition.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/global_suppression.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/list.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/map.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/marzano/complex.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/marzano/engine.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/marzano/go_code.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/marzano/go_unknown.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/marzano/python.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/marzano/react_hooks.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/marzano/shadow_scope.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/multifile.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/patterns/any.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/patterns/contains.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/patterns/create_new_files.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/patterns/definition_body.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/patterns/file_node.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/patterns/if_else.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/patterns/like.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/patterns/list_pattern.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/patterns/or.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/patterns/orelse.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/patterns/regex.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/patterns/where_pattern.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/predicates/and.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/predicates/any.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/predicates/assignment.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/predicates/or.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/sequential.grit.snapis excluded by!**/*.snapand included by**crates/biome_grit_formatter/tests/specs/grit/version.grit.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/astro/component.astro.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/astro/directives.astro.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/astro/lowercase-member.astro.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/attributes/break.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/attributes/multiline/break.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/attributes/multiline/no-break.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/attributes/no-break.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/attributes/normalize-attr-casing.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/attributes/self-closing.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/attributes/single-quotes.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/bracket-same-line/void_elements.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/comments/after-text-inside-div.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/comments/after-text.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/comments/before-text-2.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/comments/before-text-inside-div.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/comments/before-text.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/comments/inline-with-block-element.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/comments/inline.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/comments/inside-text.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/component-frameworks/astro-component-casing.astro.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/component-frameworks/html-canonical-lowercasing.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/component-frameworks/large-self-closing.svelte.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/component-frameworks/svelte-component-casing.svelte.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/component-frameworks/vue-component-casing.vue.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/content-breaks-when-attr-breaks.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/directive/lowercase.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/directive/uppercase.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/directive/uppercase2.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/elements/bracket-same-line/element.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/elements/bracket-same-line/self-closing.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/elements/empty-extra-lines.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/elements/iframe-allow.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/elements/inline/inline-before-self-closing.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/elements/inline/mixed-block-inline.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/elements/inline/tags-dont-hug-content-2.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/elements/inline/tags-dont-hug-content.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/elements/inline/tags-hug-content-longer-w-attr.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/elements/inline/tags-hug-content-w-attr.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/elements/inline/tags-hug-content.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/elements/normalize-tag-casing.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/elements/pre-with-brackets.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/elements/pre-with-braille.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/elements/pre.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/elements/spacing/case0.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/elements/spacing/case1.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/elements/spacing/case2.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/elements/spacing/case3.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/elements/spacing/case4.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/elements/whitespace/ignore/long-content.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/embedded/strict-whitespace-sensitivity/sensitive-style.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/frontmatter.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/interpolation/interpolation.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/long-content.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/long-inline-elements.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/many-children.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/self-closing.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/self-closing/always.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/self-closing/never.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/suppressions/basic-suppression-2.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/suppressions/basic-suppression.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/suppressions/global_suppression.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/suppressions/inline-content.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/suppressions/inline-elements.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/suppressions/suppress-inside-element.html.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/svelte/attach.svelte.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/svelte/await_array_destructuring.svelte.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/svelte/await_basic.svelte.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/svelte/await_catch_only.svelte.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/svelte/await_complex_expression.svelte.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/svelte/await_destructuring.svelte.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/svelte/await_dynamic_import.svelte.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/svelte/await_inline.svelte.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/svelte/await_long_expression.svelte.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/svelte/await_multiline.svelte.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/svelte/await_multiple.svelte.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/svelte/await_nested.svelte.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/svelte/await_no_catch.svelte.snapis excluded by!**/*.snapand included by**crates/biome_html_formatter/tests/specs/html/svelte/await_one_liner.svelte.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (18)
Cargo.tomlcrates/biome_css_formatter/Cargo.tomlcrates/biome_css_formatter/tests/quick_test.rscrates/biome_css_formatter/tests/spec_test.rscrates/biome_formatter_test/Cargo.tomlcrates/biome_formatter_test/src/lib.rscrates/biome_formatter_test/src/snapshot_builder.rscrates/biome_formatter_test/src/spec.rscrates/biome_graphql_formatter/Cargo.tomlcrates/biome_graphql_formatter/tests/quick_test.rscrates/biome_graphql_formatter/tests/spec_test.rscrates/biome_grit_formatter/Cargo.tomlcrates/biome_grit_formatter/tests/language.rscrates/biome_grit_formatter/tests/quick_test.rscrates/biome_grit_formatter/tests/spec_test.rscrates/biome_html_formatter/Cargo.tomlcrates/biome_html_formatter/tests/quick_test.rscrates/biome_html_formatter/tests/spec_test.rs
💤 Files with no reviewable changes (1)
- crates/biome_grit_formatter/tests/language.rs
Comment on lines
+309
to
+312
| Err(_) => return, // Re-formatting failed (e.g., file with errors), skip check | ||
| }; | ||
|
|
||
| if re_printed.as_code() != output_code && !re_printed.as_code().is_empty() { |
Contributor
There was a problem hiding this comment.
Early return and empty-output check could mask formatter bugs.
- Line 309: Returning early when re-formatting fails means if the formatter produces syntactically invalid output from valid input, the test silently passes without building a snapshot.
- Line 312: The
!re_printed.as_code().is_empty()condition means if re-formatting produces empty output, no assertion fires. An empty result from valid formatted code is likely a bug worth catching.
Consider whether these silent exits are intentional or should at least emit a warning.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_formatter_test/src/spec.rs` around lines 309 - 312, The test
currently skips assertions when re-formatting fails or produces empty output,
masking formatter regressions; in the block handling the formatter result (the
match that yields Err(_) and the subsequent check using re_printed.as_code()),
change the Err(_) => return early-return into a test failure or logged panic
that includes the error details from the formatter, and remove (or at least
replace with an explicit assertion) the !re_printed.as_code().is_empty() guard
so that empty formatted output causes the test to fail and surface the issue
(use the existing re_printed and its as_code() for the message).
Comment on lines
+10
to
12
| let Some(test_file) = SpecTestFile::try_from_file(spec_input_file, root_path) else { | ||
| panic!("Failed to set up snapshot test"); | ||
| }; |
Contributor
There was a problem hiding this comment.
Don’t turn the intentional None case into a failing test.
SpecTestFile::try_from_file returns None for options.json on purpose. Panicking here means HTML spec directories with formatter options will blow up instead of being skipped, unlike the other harnesses in this refactor.
🩹 Straightforward fix
- let Some(test_file) = SpecTestFile::try_from_file(spec_input_file, root_path) else {
- panic!("Failed to set up snapshot test");
- };
+ let Some(test_file) = SpecTestFile::try_from_file(spec_input_file, root_path) else {
+ return;
+ };Based on learnings: Create tests/specs/<language>/options.json to specify non-default formatter options for tests.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_html_formatter/tests/spec_test.rs` around lines 10 - 12, The
test currently panics when SpecTestFile::try_from_file returns None (e.g., for
options.json), so change the panic into a graceful skip: replace the `let
Some(test_file) = SpecTestFile::try_from_file(...) else { panic!(...) }` pattern
with an explicit match or if-let that returns early or continues the test
harness when None is returned (e.g., `if let Some(test_file) =
SpecTestFile::try_from_file(...) { /* run assertions */ } else { return; }`),
ensuring the harness skips spec directories that intentionally produce None
instead of failing; keep all processing logic that uses the resulting test_file
(the test_file variable / SpecTestFile) unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR is a big one, but I believe it's for the best.
The current formatter infrastructure is old and not very well maintainable. Nowadays, every time we add a new top-level option, we need to implement
Display, and all existing snapshots need updating. This isn't good for users and us maintainers.This PR refactors the infra, and it uses the
Workspacefor easy implementation. It removes theDisplayimplementation and shows it as it is.It also removes the double output. While it was useful at the beginning, I don't think it's as useful now. We have plenty of tests and if there's a regression, we should be able to catch it.
I developed the refactor with a coding agent.
Test Plan
Green CI
Docs