Skip to content

Conversation

@jonasfj
Copy link
Member

@jonasfj jonasfj commented Dec 18, 2025

A set of YAML files that we will load and then use yaml_edit to perform arbitrary edits, attempting to trigger internal errors.

This works because YamlEditor._performEdit will perform the semantic mutation on the existing YamlNode tree, and then modify the YAML string re-parse it and compare it to the semantically expected YamlNode tree.

This ensures that YamlEditor always produces valid YAML, and that the YAML has the expected semantics. The internal check in YamlEditor does not ensure that comments are preserved or that formatting is reasonable. But it does ensure that semantics are as expected, and that the YAML produced is valid.

These tests simply exercises these mechancs.

I've already added quite a few files that we need to immediately skip because we cannot perform certain mutations on them correctly. Is is particularly likely when comments are inserted in weird locations.

How many issues we actually have is hard to quantify, it's quite likely that many of the bugs are the same.


@kekavc24, I think this is an easy to way to add tests, and at-least make sure we don't have internal errors. I tried it with #2240 and it did seem to fix some of the issues. But feel free to add more for files and/or tests. It might be worth while adding small files that show specifically what we've fixed with each PR.


hmm, looking at it, I suspect I could also try to insert maps and lists in flow and block mode at each JSON path in these crash tests. Maybe, that's an exercise for later 🤣

A set of YAML files that we will load and then use yaml_edit to
perform arbitrary edits, attempting to trigger internal errors.

This works because `YamlEditor._performEdit` will perform the semantic
mutation on the existing YamlNode tree, and then modify the YAML string
re-parse it and compare it to the semantically expected YamlNode tree.

This ensures that `YamlEditor` always produces valid YAML, and that the
YAML has the expected semantics. The internal check in `YamlEditor` does
not ensure that comments are preserved or that formatting is reasonable.
But it does ensure that semantics are as expected, and that the YAML
produced is valid.

These tests simply exercises these mechancs.

I've already added quite a few files that we need to immediately skip
because we cannot perform certain mutations on them correctly.
Is is particularly likely when comments are inserted in weird locations.

How many issues we actually have is hard to quantify, it's quite likely
that many of the bugs are the same.
@jonasfj jonasfj requested a review from sigurdm December 18, 2025 10:06
@jonasfj jonasfj requested a review from a team as a code owner December 18, 2025 10:06
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a valuable set of crash tests for yaml_edit. The approach of fuzzing the YamlEditor by applying various mutations to different YAML structures is excellent for uncovering edge cases and ensuring robustness. The test implementation is well-structured, using modern Dart features effectively. I have one suggestion to improve the test code's clarity. Overall, this is a great addition to the test suite.

@github-actions
Copy link

github-actions bot commented Dec 18, 2025

PR Health

License Headers ✔️
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
Files
no missing headers

All source files should start with a license header.

Unrelated files missing license headers
Files
pkgs/bazel_worker/benchmark/benchmark.dart
pkgs/benchmark_harness/integration_test/perf_benchmark_test.dart
pkgs/boolean_selector/example/example.dart
pkgs/clock/lib/clock.dart
pkgs/clock/lib/src/clock.dart
pkgs/clock/lib/src/default.dart
pkgs/clock/lib/src/stopwatch.dart
pkgs/clock/lib/src/utils.dart
pkgs/clock/test/clock_test.dart
pkgs/clock/test/default_test.dart
pkgs/clock/test/stopwatch_test.dart
pkgs/clock/test/utils.dart
pkgs/coverage/lib/src/coverage_options.dart
pkgs/html/example/main.dart
pkgs/html/lib/dom.dart
pkgs/html/lib/dom_parsing.dart
pkgs/html/lib/html_escape.dart
pkgs/html/lib/parser.dart
pkgs/html/lib/src/constants.dart
pkgs/html/lib/src/encoding_parser.dart
pkgs/html/lib/src/html_input_stream.dart
pkgs/html/lib/src/list_proxy.dart
pkgs/html/lib/src/query_selector.dart
pkgs/html/lib/src/token.dart
pkgs/html/lib/src/tokenizer.dart
pkgs/html/lib/src/treebuilder.dart
pkgs/html/lib/src/utils.dart
pkgs/html/test/dom_test.dart
pkgs/html/test/parser_feature_test.dart
pkgs/html/test/parser_test.dart
pkgs/html/test/query_selector_test.dart
pkgs/html/test/selectors/level1_baseline_test.dart
pkgs/html/test/selectors/level1_lib.dart
pkgs/html/test/selectors/selectors.dart
pkgs/html/test/support.dart
pkgs/html/test/tokenizer_test.dart
pkgs/html/test/trie_test.dart
pkgs/html/tool/generate_trie.dart
pkgs/pubspec_parse/test/git_uri_test.dart
pkgs/stack_trace/example/example.dart
pkgs/watcher/test/custom_watcher_factory_test.dart
pkgs/yaml_edit/example/example.dart

This check can be disabled by tagging the PR with skip-license-check.

API leaks ✔️

The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.

Package Leaked API symbol Leaking sources

This check can be disabled by tagging the PR with skip-leaking-check.

Breaking changes ✔️
Package Change Current Version New Version Needed Version Looking good?
yaml_edit None 2.2.3 2.2.3 2.2.3 ✔️

This check can be disabled by tagging the PR with skip-breaking-check.

Coverage ✔️
File Coverage

This check for test coverage is informational (issues shown here will not fail the PR).

This check can be disabled by tagging the PR with skip-coverage-check.

Changelog Entry ✔️
Package Changed Files

Changes to files need to be accounted for in their respective changelogs.

This check can be disabled by tagging the PR with skip-changelog-check.

@github-actions
Copy link

Package publishing

Package Version Status Publish tag (post-merge)
package:bazel_worker 1.1.5 already published at pub.dev
package:benchmark_harness 2.4.0 already published at pub.dev
package:boolean_selector 2.1.2 already published at pub.dev
package:browser_launcher 1.1.3 already published at pub.dev
package:cli_config 0.2.1-wip WIP (no publish necessary)
package:cli_util 0.5.0-wip WIP (no publish necessary)
package:clock 1.1.3-wip WIP (no publish necessary)
package:code_builder 4.11.1 ready to publish code_builder-v4.11.1
package:coverage 1.15.0 already published at pub.dev
package:csslib 1.0.2 already published at pub.dev
package:extension_discovery 2.1.0 already published at pub.dev
package:file 7.0.2-wip WIP (no publish necessary)
package:file_testing 3.1.0-wip WIP (no publish necessary)
package:glob 2.1.3 already published at pub.dev
package:graphs 2.3.3-wip WIP (no publish necessary)
package:html 0.15.7-wip WIP (no publish necessary)
package:io 1.1.0-wip WIP (no publish necessary)
package:json_rpc_2 4.1.0-wip WIP (no publish necessary)
package:markdown 7.3.1-wip WIP (no publish necessary)
package:mime 2.1.0-wip WIP (no publish necessary)
package:oauth2 2.0.5 already published at pub.dev
package:package_config 2.3.0-wip WIP (no publish necessary)
package:pool 1.5.2 already published at pub.dev
package:process 5.0.5 already published at pub.dev
package:pub_semver 2.2.0 already published at pub.dev
package:pubspec_parse 1.5.1-wip WIP (no publish necessary)
package:source_map_stack_trace 2.1.3-wip WIP (no publish necessary)
package:source_maps 0.10.14-wip WIP (no publish necessary)
package:source_span 1.10.1 already published at pub.dev
package:sse 4.1.8 already published at pub.dev
package:stack_trace 1.12.2-wip (error) pubspec version (1.12.2-wip) and changelog (1.12.2-dev) don't agree
package:stream_channel 2.1.4 already published at pub.dev
package:stream_transform 2.1.2-wip WIP (no publish necessary)
package:string_scanner 1.4.1 already published at pub.dev
package:term_glyph 1.2.3-wip WIP (no publish necessary)
package:test_reflective_loader 0.5.0 ready to publish test_reflective_loader-v0.5.0
package:timing 1.0.2 already published at pub.dev
package:unified_analytics 8.0.11 ready to publish unified_analytics-v8.0.11
package:watcher 1.2.0 already published at pub.dev
package:yaml 3.1.3 already published at pub.dev
package:yaml_edit 2.2.3 already published at pub.dev

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

@jonasfj jonasfj merged commit 9f6c3c9 into dart-lang:main Dec 18, 2025
19 checks passed
@jonasfj jonasfj deleted the yaml_edit-crash-test branch December 18, 2025 12:06
@coveralls
Copy link

coveralls commented Dec 18, 2025

Pull Request Test Coverage Report for Build 20336075955

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 97.172%

Totals Coverage Status
Change from base Build 20336063870: 0.0%
Covered Lines: 859
Relevant Lines: 884

💛 - Coveralls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants