You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Replaces the formatting tests in Fantomas.Core.Tests with snapshot tests: every case is an F# file, and beside it the result formatting gives for it. The new project is src/Fantomas.Core.SnapshotTests, and its README is the reference.
name.fs or name.fsi is the input. Settings go in a front matter block comment on top, read by the same editorconfig code the tool uses.
name.gold.fs holds the result. An input with #if also gets a gold per define combination (name.no-defines.gold.fs, name.DEBUG.gold.fs), beside the merged one.
A mismatch fails with a line diff and writes name.actual.fs beside the gold. FANTOMAS_UPDATE_SNAPSHOTS=1, or dotnet fsi build.fsx -- -p UpdateSnapshots, accepts.
A case under negative/ must come back unchanged and has no gold. Any other case must change, and by more than its ending (a final newline added, say), so every gold shows something.
name.ignore.fs is a case for a bug that is not fixed: its gold holds what should come out, the test is skipped with the reason in its description, and it fails once it passes.
What every case checks
the result is valid F#, under every define combination;
comments are all still there, as a set and as a count, and conditional and warn directives with the same text, compared under each define combination of the input, so a comment in an #if branch counts too;
formatting is idempotent, merged and per define combination;
every node's Children are in source order;
no line ends in whitespace;
with \r\n line endings in and end_of_line = crlf, the result is the same with \r\n line endings, which is what Windows users get;
the harness, which formats each define combination itself, agrees with formatDocumentWith.
Cases under oak/ and settings/ are also checked against their folder: the node class or union case it names is in the input's Oak, and the setting it names is set and changes the result. Every union case and node class of the Oak has a folder under oak/, and every setting one under settings/, each with at least one case, and a test keeps it that way.
How the old tests became cases
By a script, not by hand, so that nothing rests on judging 3,000 tests one by one:
Every test that formats a string and compares it with an expected string became a case: its input as written, its config as front matter, its expected output as the gold. Several steps in one test, helper functions of a test file, [<TestCase>]/[<TestCaseSource>] and interpolated strings were followed too.
Before anything was written, each test's expected output was compared with what the harness gives. So every case holds what its test asserted, with one exception: 148 tests whose expected output was their input with only its end tidied (a final newline added, trailing spaces dropped) are negative cases of that expected output, since a gold would show nothing else.
2,956 tests became 2,954 cases under cases/scenarios/<old test file>/, 793 of them negative and 6 ignored. (The three Exception abbreviation loses its right-hand side #3511 tests main gained after the conversion were written as cases the same way when rebasing. long array sequence in ListTests.fs, behind #if RELEASE where the script did not look, was converted by hand, and so were the inputs of the two tests that formatted their result twice.) They stay in the folder of the file they came from: they are inputs shaped by what users ran into, often with settings that meet, and splitting them into oak/ and settings/ would lose that.
Coverage parity: every old test and every case was measured on its own against one instrumented Fantomas.Core (dotnet fsi build.fsx -- -p CoverageReach). Every point the old tests reached is reached by a case or by a test that stays. (The two [<TestCaseSource>] tests of PrefixTests.fs, 40 instances, were left out of that measurement; all their inputs are cases. scripts/reach.fsx now measures such tests, and fails on a test it cannot fill.)
The script and its ledger, one row per old test with what it became, are in this branch's history (scripts/convert.fsx, porting-ledger.tsv) and were removed with the tests. cases/scenarios/README.md says where the ledger is incomplete.
Fantomas.Core.Tests keeps 216 tests, what a case cannot show: unit tests of internals, formatting a syntax tree without its source, inputs large enough to overflow the stack, and a parse error. 9 tests that never ran (no [<Test>], or ignored with a single define combination) were dropped.
Also in this pull request
Fantomas.EditorConfig, a new project (not packed) holding the editorconfig settings code, so cases read their front matter exactly as the tool does. Finding .editorconfig files stays in Fantomas (EditorConfigFiles).
InternalsVisibleTo for the snapshot project, the only change to Fantomas.Core.
Scripts reference the snapshot project: scripts/format.fsx runs an input through every check above, and with --define A,B prints one define combination before the merge. scripts/trivia.fsx shows where trivia landed.
Pipelines: UpdateSnapshots, SnapshotReports (which optional parts and trivia each node class has cases for), CoverageOak (coverage of SyntaxOak.fs), CoverageReach (coverage per test). Coverage counts the snapshot tests towards Fantomas.Core, and measures Fantomas.EditorConfig with Fantomas.Tests.
Contributor docs explain how to write a case and how to debug a failed merge of define combinations.
Not in this pull request
A script that lists the changed lines of Fantomas.Core no case reaches, built on CoverageReach, as guidance for new work. A follow-up.
Fantomas.Core.Tests repeats the same format-and-compare structure in
over 3,000 tests, grouped by history more than by what they cover.
Fantomas.Core.SnapshotTests keeps formatting tests as files instead: an
F# input under cases/, with an optional block comment front matter of
editorconfig settings, and gold files beside it holding the result.
cases/ is laid out by the Oak: oak/<Union>/<Case>/ for a node,
settings/<key>/ shaped the same way below it for a setting, and a
trivia/ folder per node for the comments, blank lines and directives
attached to it. The path is checked against each case: the folder's
node must be in its Oak, a settings case must set its setting and the
setting must change the result, and a trivia case must attach trivia to
its node.
Every case is checked as the old helpers did, for a valid and idempotent
result that keeps every comment, and against formatDocumentWith. A case
with #if gets a gold per define combination besides the merged one,
which replaces the tests that formatted a single combination. A result
that fails a check is never written as a gold, not even when updating.
The pilot ports the union and union case tests: 49 old tests ported,
10 merged into other cases and 3 dropped, in porting-ledger.tsv.
scripts/ledger.fsx generates that ledger from the old test files and
finds the old tests that touch a node. build.fsx gains UpdateSnapshots,
CoverageOak, which measures SyntaxOak.fs coverage by the cases, and
SnapshotReports, which writes which node shapes and trivia positions the
cases cover.
Front matter is read by the code the tool uses, so that code moves into
a new Fantomas.EditorConfig project, together with Suggestion, which it
depends on. The CLI keeps reading .editorconfig files from disk, in a
module renamed to Fantomas.EditorConfigFiles so that two assemblies do
not define the same module.
oak/ModuleDecl/Exception/ is the first folder of the port: 8 old tests
ported, 1 merged, and new cases for what the folder still lacked, such
as several members, long fields and comments around the access
modifier. ExceptionDefnNode is now reached in full by the cases.
A case under settings/<key>/negative/ is one the setting must leave
alone. It has no gold: its input is already formatted, and formatting it
with the setting and with the setting at its default must both give it
back unchanged. The first is an exception, which never gets the bar
fsharp_bar_before_discriminated_union_declaration puts before a single
union case.
Which node a case's trivia attaches to is no longer checked. That is how
Trivia.fs works today and may change without the formatting changing.
What is checked is that nothing is lost: besides comments, the result
must have as many conditional directives and warn directives as the
input, as both are trivia rather than part of the syntax tree. Three
union case comment cases that sat with the field types for that check's
sake move back to oak/UnionCase/trivia/.
Every problem a case can have is a Problem value instead of a sentence,
so a result that is itself broken (invalid, not idempotent, a lost
comment) is never written as a gold, not even when updating. The
trailing whitespace check no longer flags lines that end inside a
triple quoted string or a block comment, where the whitespace is
content.
The diagnostic scripts reference the built snapshot test assembly, so
format.fsx runs the harness's own checks and every script takes a case
file with its front matter as settings. trivia.fsx is new and lists
where each piece of trivia landed. scripts/ledger.fsx gains --input,
which prints an old test's input. The reports are written at the end of
a test run when SnapshotReports asks for them, rather than by a test of
their own, and the coverage summary names the classes reached in full.
oak/ModuleDecl/ExternBinding/ is the next folder of the port: 18 old
tests ported, 3 merged, and the 2 about extern members left for
oak/MemberDefn/ExternBinding/. ExternBindingNode and
ExternBindingPatternNode are now reached in full by the cases.
Close to half the cases had a gold identical to their input, which
pins down nothing a gold could add. A case now either earns its gold,
the result differing from the input, or sits in a negative/ folder,
last below its node folder, where formatting must leave the input as it
is and the case is its own gold. A negative case with #if keeps its
per-define golds, since what each combination printed is not its input.
The harness fails a case outside negative/ whose result is its input,
and one inside negative/ whose result is not.
Four cases got an input that formatting changes. The rest of those that
only repeated their input are about something formatting must leave
alone, a comment, a bar or an attribute staying where it is, and moved
to negative/ folders, together with the ledger rows pointing at them.
The README now also says how to choose between the two, that the checks
do not see a comment that moved, and how the ledger records several
cases for one test, several tests for one case, and formatAST tests.
oak/ModuleDecl/TopLevelBinding/ holds what is specific to a let at
module level, as every binding is printed by the same genBinding: the
layout of a let rec ... and group, where a long binding gets a blank
line on both sides and a comment can take that blank line's place, and
a module-level let ... in, whose next line stays a declaration of its
own. One old test is ported; the 16 others the search turned up belong
to the Expr, Binding and settings folders, and the ledger says which.
When a test overrides a file that redefines config, the ledger's config
column now names that file's config too, not only the override. The
README notes that the input --input writes keeps the newline the old
tests' strings start with, which a negative case has to lose.
oak/ModuleDecl/ModuleAbbrev/ covers module L = List in implementation
and signature files: spacing around =, an alias written on the next
line, a long alias that has nowhere to break, comments above and after
the abbreviation, and issue 2792, an abbreviation followed by a nested
module in a signature file. Both old tests are ported.
A comment right after the = is left out: the syntax tree has no range
for that =, so the comment moves.
Porting folder by folder, with each old test judged and rewritten, left
thousands of decisions nobody could review, and no way to show that no
test was lost. scripts/convert.fsx converts the old tests instead,
without judging any.
Each test that formats a string literal and compares the result with an
expected string becomes a case under cases/ported/<old test file>/: its
input as written, its config as front matter, the harness result as its
golds. Before anything is written, every expected output is compared with
the harness result, so the cases hold exactly what the old tests asserted.
2,919 tests became 2,873 cases, 617 of them negative. The 248 tests that
do not fit, unit tests among them, are listed with the reason in the
porting ledger, which the converter now writes. `--check` fails when
cases/ported/ or the ledger drift from the old tests.
The hand ports of old tests are deleted, since ported/ holds those tests
now; the hand-written cases no old test is behind stay. The placement
check accepts ported/, where only negative/ means anything, and
scripts/ledger.fsx goes, replaced by the converter.
A case for a bug that is not fixed yet is now `name.ignore.fs`. Its gold
holds what formatting should give, its `#` description says why it is
ignored, and a run skips it with that reason. Once it gives its golds,
it fails and asks to drop the `.ignore`, so a fixed bug cannot stay
hidden behind one. Nothing writes the golds of an ignored case, not even
the update mode.
The converter now checks `[<Ignore>]` tests like any other, so one that
passes is converted as usual. One that does not becomes an ignored case,
with the test's reason and its expected output as the gold, unless it
formats a single define combination, whose output no gold can hold.
`--check` leaves the `.actual` files that ignored cases write alone.
The CoverageReach pipeline measures what each old test and each snapshot
case reaches in Fantomas.Core, one at a time, against a single
instrumented build, and reports what the old tests reach that no case
does. AltCover's own per-test tracking loses the test at the first async
hop, so scripts/reach.fsx calls the tests itself and clears the
recorder's visits and samples between them, after running every module
initialiser so no test gets those to itself. Parity holds: every point
the old tests reach is reached by a snapshot case or by an old test that
stays.
The converter now also reads tests that go through a helper function of
their file, take several steps, are parameterised, build their input
with interpolated strings, or format a result again. Each step or
argument becomes a case, verified against the old expected output like
the rest. What stays unconverted formats a syntax tree without source,
checks for a stack overflow on a generated input, or expects an
exception.
Every formatting test in Fantomas.Core.Tests now lives as a snapshot
case under cases/ported/, so the old test files go. Before they went,
the converter checked once more that each case holds exactly what its
test expected, and the per-test coverage showed that nothing the old
tests reached is lost. The converter also picked up the tests in
BlankLinesAroundNestedMultilineExpressions.fs, which its file name
pattern had missed; it now reads the files from the project.
Fantomas.Core.Tests keeps what a case cannot show: the unit tests of
internals, formatting a syntax tree without its source, inputs large
enough to overflow the stack, and a parse error. Those few tests move
into FormatAstTests.fs, a new StackOverflowTests.fs and
CodeFormatterTests.fs. Tests that never ran are dropped. The converter
and its ledger are removed with the tests they described.
format.fsx --define prints one define combination before the merge,
which takes over from formatSourceStringWithDefines when debugging a
failed merge. The Coverage pipeline counts the snapshot tests towards
Fantomas.Core, CoverageReach measures the unit tests and the cases, and
the contributor docs explain how to write a case.
Update mode skipped the golds of a case whose result was broken, but
still wrote them, and deleted stale ones, for a case that failed its
folder's checks: a folder naming a node its input lacks, or a case
outside negative/ that formatting leaves unchanged. The run then failed
on the placement anyway and left a gold behind that should not exist.
It now leaves the golds alone until the case is where it belongs, as
the README already said it did.
A gold was read as text, which drops a UTF-8 byte order mark, so a gold
saved with one by an editor still matched and was never rewritten. Golds
are now compared as bytes against the result written as UTF-8 without a
mark, and a gold that differs only by its mark fails with a note saying
so instead of an empty line diff. Ignored cases compare the same way.
The new harness only compares the number of conditional and warning directives. It does not compare their contents as the PR description promises, so a formatting bug that changes #if DEBUG to another condition or rewrites a #nowarn code can be accepted by UpdateSnapshots as long as the count stays constant. Please collect and compare normalized directive text as a set as well as comparing the count, analogous to the comment-preservation check.
Comments and directives were only compared under no defines, so a comment
or #nowarn in an #if branch could go missing unnoticed. They are now
compared under every define combination of the input, and directives by
their text, not just by how many there are.
The old tests ran with \r\n line endings on Windows; the cases only ever
saw \n. A case is now also formatted with \r\n in and end_of_line = crlf,
and must give the same result with \r\n line endings.
A union case folder such as Expr/Lambda only asked for its node class,
which other nodes hold too: (fun x -> x) passed as a lambda. It now asks
for the union case itself in the Oak, which also makes the folders that
were left unchecked checkable.
A case whose result only adds a final newline earned a gold that showed
nothing else. That is no longer accepted, unless the case sets
insert_final_newline, and the 148 ported cases that did so are now
negative cases that end the way the result does.
Smaller fixes: an ignored case reports what its folder asks of its input;
front matter setting unset, indent_size = tab or max_line_length = off
fails; the scripts format a case without front matter with \n like the
tests do; AnalyzeChanged leaves cases and golds alone; the stray-file
test no longer throws on a name without a dot and passes over hidden
files; and the multiple defines guide shows the output the example
really gives.
The Oak facts only read the properties a node class declares itself, so
the fields record nodes inherit from ExprRecordBaseNode were invisible:
a case in ExprRecordFieldOrSpread/Field, or a parenthesised copy
expression in Expr/Paren, failed its folder check, and the shape and
trivia reports skipped those fields. Inherited Oak properties now count.
An ignored case now reports what is true of it whatever formatting
gives, before formatting and even when that throws: a setting its
folder names that it does not set, a gold beside it under negative/, or
a gold for a define combination it does not have.
The final newline rule let any insert_final_newline in the front matter
through, the default value included; only a value other than the
default does now.
The scripts read stdin with \n line endings, as a case file is read,
which they need now that every case is also formatted with \r\n, and
--editorconfig content gets end_of_line = lf unless it sets one. The
multiple defines guide puts its example in Expr/Chain, which is what
the example is.
A case under settings/ could set its key to the default and pass under
negative/, where nothing else notices: it now fails with SettingAtDefault.
A setting with named values now needs a value folder the setting can
take, and says so, instead of reading the next folder as a node.
settings/ held no case, so its checks ran on nothing: three cases now
cover a setting, a value folder and a negative case.
The rest is what no longer said what the code does:
- the README dropped a front matter warning that did not hold, says
when an ignored case skips its checks, and that the end of a file at
the default settings is for a unit test, not a case;
- build.fsx stopped reading coverage-exceptions.tsv, which never
existed, and counts four coverage results;
- reach.fsx skips ignored cases, as it skips ignored unit tests;
- Claim.IsTrivia, breaksResult and a second copy of the SyntaxOak types
went, as nothing needed them, and the doc comment of check is back on
check.
A case at end_of_line = crlf skipped the line ending check, which only
ran at lf, so nothing verified it: formatting its input with \r\n line
endings now has to give its result back, and a first such case pins it.
Directives were compared sorted, so one that moved past another passed,
while merging the define combinations relies on their order: they are
compared in order now.
A case that cannot be read fails with what is wrong rather than an
exception, a folder naming an abstract node class says it is abstract
rather than that the node is missing, and a value that differs from its
folder names the whole settings path.
The scripts stop on a path that does not exist, where they read stdin
and printed nothing, and format.fsx reports a formatting error on stderr
with exit code 1 rather than as its output. The docs say their 2844 case
path is an example to create, and the doc comment of
isSpecDefinedNonValue lives in the signature file only.
The tests Fantomas.Core.Tests had are not a backlog to sort into oak/
and settings/: they are inputs shaped by what users ran into, often
with settings that meet, and splitting them would lose that. They now
live in scenarios/, named for what they are rather than where they
came from.
scenarios/README.md says where they come from, how a test became a
case, and how to get their history back through the old file paths.
The comments the old test files had around their tests, which the
conversion left behind, are in a README.md per folder, each with the
cases it was written above or inside. The check for stray files lets a
README.md sit among the cases.
oak/ now has a folder with at least one case for every union case of
the Oak and every node class no union case holds, and settings/ one
for every setting. They are a starting point for the cases fixes add.
oak/README.md names the three nodes no case can hold. Two of the new
cases are ignored, because formatting loses code there today: the
static of an extern inside a type, and the struct of a static
optimization constraint.
This branch has not been deployed
No deployments
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
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.
Replaces the formatting tests in
Fantomas.Core.Testswith snapshot tests: every case is an F# file, and beside it the result formatting gives for it. The new project issrc/Fantomas.Core.SnapshotTests, and its README is the reference.A case
name.fsorname.fsiis the input. Settings go in a front matter block comment on top, read by the same editorconfig code the tool uses.name.gold.fsholds the result. An input with#ifalso gets a gold per define combination (name.no-defines.gold.fs,name.DEBUG.gold.fs), beside the merged one.name.actual.fsbeside the gold.FANTOMAS_UPDATE_SNAPSHOTS=1, ordotnet fsi build.fsx -- -p UpdateSnapshots, accepts.negative/must come back unchanged and has no gold. Any other case must change, and by more than its ending (a final newline added, say), so every gold shows something.name.ignore.fsis a case for a bug that is not fixed: its gold holds what should come out, the test is skipped with the reason in its description, and it fails once it passes.What every case checks
#ifbranch counts too;Childrenare in source order;\r\nline endings in andend_of_line = crlf, the result is the same with\r\nline endings, which is what Windows users get;formatDocumentWith.Cases under
oak/andsettings/are also checked against their folder: the node class or union case it names is in the input's Oak, and the setting it names is set and changes the result. Every union case and node class of the Oak has a folder underoak/, and every setting one undersettings/, each with at least one case, and a test keeps it that way.How the old tests became cases
By a script, not by hand, so that nothing rests on judging 3,000 tests one by one:
[<TestCase>]/[<TestCaseSource>]and interpolated strings were followed too.cases/scenarios/<old test file>/, 793 of them negative and 6 ignored. (The three Exception abbreviation loses its right-hand side #3511 testsmaingained after the conversion were written as cases the same way when rebasing.long array sequenceinListTests.fs, behind#if RELEASEwhere the script did not look, was converted by hand, and so were the inputs of the two tests that formatted their result twice.) They stay in the folder of the file they came from: they are inputs shaped by what users ran into, often with settings that meet, and splitting them intooak/andsettings/would lose that.dotnet fsi build.fsx -- -p CoverageReach). Every point the old tests reached is reached by a case or by a test that stays. (The two[<TestCaseSource>]tests ofPrefixTests.fs, 40 instances, were left out of that measurement; all their inputs are cases.scripts/reach.fsxnow measures such tests, and fails on a test it cannot fill.)The script and its ledger, one row per old test with what it became, are in this branch's history (
scripts/convert.fsx,porting-ledger.tsv) and were removed with the tests.cases/scenarios/README.mdsays where the ledger is incomplete.Fantomas.Core.Testskeeps 216 tests, what a case cannot show: unit tests of internals, formatting a syntax tree without its source, inputs large enough to overflow the stack, and a parse error. 9 tests that never ran (no[<Test>], or ignored with a single define combination) were dropped.Also in this pull request
Fantomas.EditorConfig, a new project (not packed) holding the editorconfig settings code, so cases read their front matter exactly as the tool does. Finding.editorconfigfiles stays inFantomas(EditorConfigFiles).InternalsVisibleTofor the snapshot project, the only change toFantomas.Core.scripts/format.fsxruns an input through every check above, and with--define A,Bprints one define combination before the merge.scripts/trivia.fsxshows where trivia landed.UpdateSnapshots,SnapshotReports(which optional parts and trivia each node class has cases for),CoverageOak(coverage ofSyntaxOak.fs),CoverageReach(coverage per test).Coveragecounts the snapshot tests towards Fantomas.Core, and measuresFantomas.EditorConfigwithFantomas.Tests.Not in this pull request
CoverageReach, as guidance for new work. A follow-up.