[mustache_template] Fix auto-generated specification tests, run with `dart test` by jslater89 · Pull Request #11056 · flutter/packages

jslater89 · 2026-02-18T18:13:50Z

This PR reorganizes the tests in mustache_template such that CI can run them, per the second option proposed in flutter/flutter#174721. The changes include a script to pull mustache specifications from the mustache repository and note the time/date and commit hash of that action, along with documentation of the above and the new test structure laid out below.

The former all.dart (which called three main functions in three test files) is now mustache_test.dart, and the former mustache_test.dart (which contained handwritten tests) is now feature_test.dart. The mustache_specs.dart file is now solely responsible for generating the tests in mustache_test.dart from the mustache specs, and no longer has a main of its own.

As this implementation doesn't support two of the optional mustache specs (inheritance and dynamic names, which were added to the mustache repo in the early 2020s, while the original mustache_template package was written in the mid-2010s), this PR also adds an UNSUPPORTED_SPECS constant in mustache_test.dart that skips generation of tests from those spec files.

(Separately, I'm working on an implementation of the inheritance spec, so it seemed better to pull all the specification files and selectively disable the unsupported ones.)

Pre-Review Checklist

Exemptions

Version change exemption: PR affects tests only.
CHANGELOG exemption: PR affects tests only.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩² ↩³

Calling the other 'main' functions would run all the tests from parser_test and feature_test from mustache_test.dart as well as their standalone files.

gemini-code-assist

Code Review

This pull request is a significant and valuable refactoring of the mustache_template test suite. By reorganizing the tests to be runnable with dart test, introducing a script to manage the mustache specifications, and cleaning up the test structure, you've greatly improved the maintainability and usability of the tests. The changes are well-documented in the new README.md. I have a couple of suggestions to further improve the new download script and enhance consistency within the test code.

gemini-code-assist · 2026-02-18T18:16:06Z

third_party/packages/mustache_template/test/download_spec.sh

+git clone https://github.com/mustache/spec.git tmp_spec
+
+HEAD_HASH=$(git -C tmp_spec rev-parse HEAD)
+UTC_NOW=$(date -u +%Y-%m-%dT%H:%M:%SZ)
+
+if [ -d "specs" ]; then
+    rm -rf specs
+fi
+mkdir -p specs
+
+mv tmp_spec/specs/* specs/
+rm -rf tmp_spec
+
+echo "Specs commit: $HEAD_HASH" > specs/meta.txt
+echo "Specs download date: $UTC_NOW" >> specs/meta.txt


This script is a great addition for managing the spec files. To make it more robust and slightly simpler, I suggest adding a shebang and set -e at the top. The shebang ensures the script is run with the correct interpreter, and set -e will cause the script to exit immediately if any command fails, preventing potential issues.

I've also simplified the directory removal logic, as rm -rf doesn't error if the directory doesn't exist, making the if check redundant.

Suggested change

git clone https://github.com/mustache/spec.git tmp_spec

HEAD_HASH=$(git -C tmp_spec rev-parse HEAD)

UTC_NOW=$(date -u +%Y-%m-%dT%H:%M:%SZ)

if [ -d "specs" ]; then

rm -rf specs

fi

mkdir -p specs

mv tmp_spec/specs/* specs/

rm -rf tmp_spec

echo "Specs commit: $HEAD_HASH" > specs/meta.txt

echo "Specs download date: $UTC_NOW" >> specs/meta.txt

#!/bin/bash

set -e

git clone https://github.com/mustache/spec.git tmp_spec

HEAD_HASH=$(git -C tmp_spec rev-parse HEAD)

UTC_NOW=$(date -u +%Y-%m-%dT%H:%M:%SZ)

rm -rf specs

mkdir -p specs

mv tmp_spec/specs/* specs/

rm -rf tmp_spec

echo "Specs commit: $HEAD_HASH" > specs/meta.txt

echo "Specs download date: $UTC_NOW" >> specs/meta.txt

gemini-code-assist · 2026-02-18T18:16:06Z

third_party/packages/mustache_template/test/feature_test.dart

+      expect(ex is TemplateException, isTrue);
+      expect(
+        (ex! as TemplateException).message,
+        startsWith(BAD_VALUE_INV_SECTION),
+      );


For consistency with other tests in this file (e.g., 'Mismatched tag' on line 396), you can use the existing expectFail helper function here. This will make the test more concise and readable by abstracting the exception checking logic.

Suggested change

expect(ex is TemplateException, isTrue);

expect(

(ex! as TemplateException).message,

startsWith(BAD_VALUE_INV_SECTION),

);

expectFail(ex, null, null, BAD_VALUE_INV_SECTION);

stuartmorgan-g · 2026-02-24T19:20:20Z

All existing and new tests are passing.

While waiting for review, FYI unit tests are failing in CI on several platforms, which will need to be addressed.

third_party/packages/mustache_template/test/feature_test.dart

jslater89 · 2026-02-24T20:10:26Z

Whoops, so they are.

The web tests are failing because the specification test generator relies on dart:io; that would have been broken before too. I'll still fix that if needed.

The Windows failure is probably me being a dope about path separators.

I'll correct the formatting issues too when I do the others, some night this week.

stuartmorgan-g · 2026-02-24T20:24:17Z

The web tests are failing because the specification test generator relies on dart:io; that would have been broken before too.

Except that the stated purpose of the PR is:

This PR reorganizes the tests in mustache_template such that CI can run them

If something wasn't run in CI before, then it wasn't broken in CI before.

I'll still fix that if needed.

I'm not sure what you mean by "if needed". Isn't the goal to land the PR? We can't land a PR that breaks CI.

The handwritten 'null inverted section' test expected no output from {{^section}} content {{/section}} when {'section': null} in the context, but specs/inverted/Null is falsey indicates that the content should render in that case.

Lenient mode allows almost everything, but not empty tag names. {{>}} is an empty tag name: it asks for a partial with the name 'empty string'. All tags whose contents begin with a tag type sigil should be parsed as a tag of that type. {{ a> }} is a valid tag name and passes a new test. Tests are also included for empty tag names of all types.

jslater89 · 2026-02-25T16:38:53Z

Since the last review:

All tests passing on all platforms. The download-specs script now generates Dart files that contain strings containing the JSON specification files, so there's no dart:io dependency in the tests anymore.
Expects-exception tests refactored to use throwsA.
Tests commented out during the original library import reviewed, along with TODOs in tests. Some were edited:
- feature_test.dart@258: {{ > }} is not a valid tag even in lenient mode: it's an open-partial tag with an empty name, and empty names are invalid in both modes. On the other hand, {{ a> }} is a valid name in lenient mode—identifiers can include characters that would scan as sigil tokens, but the open-delimiter sigil sequence always opens a tag.
- feature_test.dart@503: the commented test labeled 'known failure' conflicted with one of the Mustache spec tests (null is falsey and should cause an inverted section to render), so it was removed.
- 'should fail with exception' tests refactored to use throwsA.

bkonyi

Just a couple of comments, but this mostly looks good.

This will need a minor version bump in the pubspec.yaml, a corresponding CHANGELOG.md entry, and feature_test.dart needs to be formatted.

bkonyi · 2026-03-04T15:43:19Z

third_party/packages/mustache_template/test/mustache_specs.dart

-        _defineGroupFromFile(filename, text);
-      }
+void defineTests(List<String> unsupportedSpecs) {
+  for (final MapEntry<String, String> entry in SPECS.entries) {


Nit: this could use object destructuring instead

for (final MapEntry(key: specName, value: text) in SPECS.entries) { // ... }

You know, I'd forgotten that was a context where patterns work.

Resolved in 168e220.

bkonyi · 2026-03-04T15:45:04Z

third_party/packages/mustache_template/test/specs/comments.json

@@ -0,0 +1,106 @@
+{


Do we need all of the JSON, Dart, and YAML files for each of these?

Since we're only consuming the *.dart files, we probably don't need to commit the original *.json and *.yml files pulled from the repo. Can we add them to the .gitignore?

Fair point!

Since the download script puts them in a temporary directory anyway, I just kept the Dart files only in b0363cf.

stuartmorgan-g · 2026-03-04T15:59:27Z

This will need a minor version bump in the pubspec.yaml, a corresponding CHANGELOG.md entry

test/ changes don't need versions or changelogs by repo policy. The failure is just a false positive because we don't happen to have .gitignore on the allow-list of known exempt files.

bkonyi · 2026-03-04T16:22:43Z

This will need a minor version bump in the pubspec.yaml, a corresponding CHANGELOG.md entry

test/ changes don't need versions or changelogs by repo policy. The failure is just a false positive because we don't happen to have .gitignore on the allow-list of known exempt files.

Ah makes sense, thanks for clarifying! Good to know those labels are there for the future.

jslater89 · 2026-03-09T19:59:15Z

Whoops, the commits I mentioned in the comments above were in my inheritance feature branch. They're in this PR now.

bkonyi

LGTM!

auto-submit · 2026-03-16T16:20:22Z

autosubmit label was removed for flutter/packages/11056, because This PR has not met approval requirements for merging. The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.

Merge guidelines: A PR needs at least one approved review if the author is already part of flutter-hackers or two member reviews if the author is not a flutter-hacker before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

parlough

Thanks for fixing and documenting this test setup. Looks good to me, with some minor suggestions to consider:

third_party/packages/mustache_template/test/download_spec.sh

third_party/packages/mustache_template/test/mustache_test.dart

third_party/packages/mustache_template/test/parser_test.dart

third_party/packages/mustache_template/test/README.md

third_party/packages/mustache_template/test/mustache_test.dart

third_party/packages/mustache_template/test/README.md

Co-authored-by: Parker Lougheed <parlough@gmail.com>

jslater89 · 2026-04-12T15:39:32Z

Sorry for the long delay; work got busy. I rewrote the shell script as a Dart script, and made the style changes from @parlough's review.

jslater89 added 4 commits February 18, 2026 10:42

Test runner script, including unsupported optional spec handling

0f57ba0

Move spec tests into a Dart test for packages infrastructure

c93d33b

Don't duplicate tests in mustache_test.dart

ccbe6e3

Calling the other 'main' functions would run all the tests from parser_test and feature_test from mustache_test.dart as well as their standalone files.

Test README documenting specification source/update process

5902256

jslater89 requested review from bkonyi and parlough as code owners February 18, 2026 18:13

gemini-code-assist bot reviewed Feb 18, 2026

View reviewed changes

jslater89 mentioned this pull request Feb 21, 2026

[mustache_template] Add support for ~inheritance optional specification flutter/flutter#182694

Open

bkonyi reviewed Feb 24, 2026

View reviewed changes

third_party/packages/mustache_template/test/feature_test.dart Outdated Show resolved Hide resolved

jslater89 added 2 commits February 24, 2026 15:51

Package specs for non-dart:io consumption, unskip passing test

31e5051

Run dart format

ec7e457

github-actions bot added the p: mustache_template label Feb 24, 2026

jslater89 added 4 commits February 24, 2026 16:05

Remove unused dart:io import

ce2a56c

Remove invalid test from feature_test.dart

beb03db

The handwritten 'null inverted section' test expected no output from {{^section}} content {{/section}} when {'section': null} in the context, but specs/inverted/Null is falsey indicates that the content should render in that case.

Refactor exception tests to use throwsA

0ad64c5

jslater89 requested a review from bkonyi February 25, 2026 16:20

bkonyi reviewed Mar 4, 2026

View reviewed changes

stuartmorgan-g added override: no versioning needed Override the check requiring version bumps for most changes override: no changelog needed Override the check requiring CHANGELOG updates for most changes labels Mar 4, 2026

jslater89 added 3 commits March 9, 2026 15:57

Only keep generated Dart files from download_spec.sh

6441425

Object destructuring for specs map entries

6d95882

Format feature_test.dart

ff6d3a6

jslater89 requested a review from bkonyi March 9, 2026 19:59

bkonyi approved these changes Mar 16, 2026

View reviewed changes

bkonyi added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 16, 2026

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 16, 2026

parlough approved these changes Mar 21, 2026

View reviewed changes

jslater89 and others added 4 commits April 12, 2026 11:33

Download_spec Dart script in place of download_spec.sh

0690a5a

Apply suggestions from code review

eca7dbe

Co-authored-by: Parker Lougheed <parlough@gmail.com>

Merge branch 'main' into spec-test-runner

2f18f33

test/README changes for specification metadata

d96b408

Lower camel case for unsupported specs constant

3b96a7b

Conversation

jslater89 commented Feb 18, 2026

Pre-Review Checklist

Exemptions

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g commented Feb 24, 2026

Uh oh!

Uh oh!

jslater89 commented Feb 24, 2026

Uh oh!

stuartmorgan-g commented Feb 24, 2026

Uh oh!

jslater89 commented Feb 25, 2026

Uh oh!

bkonyi left a comment

Choose a reason for hiding this comment

Uh oh!

bkonyi Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

jslater89 Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

bkonyi Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

jslater89 Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g commented Mar 4, 2026

Uh oh!

bkonyi commented Mar 4, 2026

Uh oh!

jslater89 commented Mar 9, 2026

Uh oh!

bkonyi left a comment

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented Mar 16, 2026

Uh oh!

parlough left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jslater89 commented Apr 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants