source_gen_test icon indicating copy to clipboard operation
source_gen_test copied to clipboard

Add ShouldGenerateFile annotation

Open alexeyinkin opened this issue 2 years ago • 16 comments

This PR adds ShouldGenerateFile annotation to expect the match of the generated code against the content of an existing file.

  • Resolves #31

Rationale

For an example use, see this package: https://github.com/alexeyinkin/dart-enum-map/tree/main/enum_map_gen (made to use a fork of source_gen_test for now)

The package generates code of 160+ lines, so to verify the output it is useful to not only test that the output matches a string, but also to write tests on that output itself.

This file uses the new ShouldGenerateFile annotation: https://github.com/alexeyinkin/dart-enum-map/blob/main/enum_map_gen/test/unmodifiable_enum_map/src/input.dart

This is the golden that the output is compared with: https://github.com/alexeyinkin/dart-enum-map/blob/main/enum_map_gen/test/unmodifiable_enum_map/src/output.dart

This is the test on that golden: https://github.com/alexeyinkin/dart-enum-map/blob/main/enum_map_gen/test/unmodifiable_enum_map/unmodifiable_enum_map_test.dart

This pair of tests ensures that the output both matches the desired one and works as expected.

Implementation

The new annotation has optional arguments partOf and partOfCurrent.

partOfCurrent adds to the output file a link back to the input file, so that the input file can have tests written for it, as in the example above.

partOf adds to the output file a link to an arbitrary file, if for some reason tests cannot be written for the input file. I can imagine cases where the input file is suitable for LibraryReader but not for actual execution, so this may be useful. Although if anyone really wants tests on their output they better refactor their input to be buildable. If in doubt, this functionality can be removed.

If both arguments are omitted, the output file is just the generated content with no additions. This is useful if no tests are planned on the output but it is just too large to be inlined in the input file.

Open Questions

~~1. Definition of 'golden'~~

~~In the wild, a 'golden' test seems to mean comparison to a known result, most often in separate files. This sense of 'golden' is used in Flutter with its --update-goldens flag (it is not called --update-golden-files). But current ShouldGenerate may also be said to compare with a golden that is just being an inline string. Is it OK to name the new annotation ShouldGenerateGolden or should it be ShouldGenerateFile or anything else?~~

The annotation renamed to ShouldGenerateFile.

2. Updating golden files

There is a need to automatically generate golden files. Flutter does this with --update-goldens flag. If it is set, then the output is not tested against goldens, but they are created or updated with a test's output.

The pure Dart does not have this concept, so we cannot use this flag to decide if we need to compare or update goldens.

Ideally we want Dart to also have a concept of golden files and to have a similar flag. Then we could have tapped into it and compare or update accordingly. The concept of golden tests is actually wider than what Flutter assumes (in Flutter it only means screenshot matches), so making Dart aware of golden files feels natural. I believe there are other applications for this in Dart like testing JSON serializers with large output, etc. Maybe we can file a feature request for this in Dart SDK repo?

For now, to generate the output I introduced the support for SOURCE_GEN_TEST_UPDATE_GOLDENS environment variable. If it is set to '1', the goldens are updated. This seems to be the easiest way to tell the comparison from the update without touching the code.

Is this OK or should we do something else to update the files?

alexeyinkin avatar Sep 18 '22 08:09 alexeyinkin