Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add end-to-end tests for Mustachio AOT compiler #2664

Merged
merged 12 commits into from Jun 3, 2021

Conversation

srawlins
Copy link
Member

@srawlins srawlins commented Jun 2, 2021

These tests, in aot_compiler_render_test.dart, are a little irregular. The goal
is to write short test cases in which we specify the text of a Mustache
template, and the expected rendered text, visually near each other, which is a
challenge, as there are several steps to get from Mustache template text to
rendered output, including running the builders, and executing freshly
generated Dart code.

The solution here is a test system which takes a Mustache template text, runs
the mustachio AOT compiler to generate a Dart script which renders a context
object into that template, then executes the generated Dart script, and
asserts on the output.

Writing these tests revealed a few bugs that are fixed concurrently:

  • imports in the generated script should be accurate; this requires using
    code_builder, which requires a sizeable change to the AOT compiler code.
    code_builder allows you to "reference" symbols like types or top-level
    functions and the URL where they may be found.
  • handle partial template paths with dots and slashes.

Additionally, fix some nits in runtime_renderer_builder_test.dart and
runtime_renderer_render_test.dart.

These tests, in aot_compiler_render_test.dart, are a little irregular. The goal
is to write short test cases in which we specify the text of a Mustache
template, and the expected rendered text, visually near each other, which is a
challenge, as there are several steps to get from Mustache template text to
rendered output, including running the builders, and executing freshly
generated Dart code.

The solution here is a test system which takes a Mustache template text, runs
the mustachio AOT compiler to generate a Dart script which renders a context
object into that template, then executes the  generated Dart script, and
asserts on the output.

Writing these tests revealed a few bugs that are fixed concurrently:

* imports in the generated script should be accurate; this requires using
  code_builder, which requires a sizeable change to the AOT compiler code.
  code_builder allows you to "reference" symbols like types or top-level
  functions and the URL where they may be found.
* handle partial template paths with dots and slashes.

Additionally, fix some nits in runtime_renderer_builder_test.dart and
runtime_renderer_render_test.dart.
@google-cla google-cla bot added the cla: yes Google CLA check succeeded. label Jun 2, 2021
@srawlins srawlins requested a review from jcollins-g June 2, 2021 16:07
@coveralls
Copy link

coveralls commented Jun 2, 2021

Coverage Status

Coverage remained the same at 58.316% when pulling 15605e9 on srawlins:aot-compiler-2 into 0922660 on dart-lang:master.

Copy link
Contributor

@jcollins-g jcollins-g left a comment

Choose a reason for hiding this comment

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

I like the direction this has taken! Something seems to be wrong with windows paths, though. Assuming that gets fixed, approved.

@kevmoo
Copy link
Member

kevmoo commented Jun 3, 2021

@@ -18,6 +16,10 @@ void main() {
var dartdocLibUri = await Isolate.resolvePackageUri(
Uri.parse('package:dartdoc/dartdoc.dart'));
var dartdocPath = p.dirname(p.dirname(dartdocLibUri.path));
// Correct Windows issue path coming out of [Isolate.resolvePackageUri].
if (dartdocPath.startsWith(p.windows.separator)) {
dartdocPath = dartdocPath.substring(1).replaceAll('/', p.separator);
Copy link
Member

Choose a reason for hiding this comment

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

so weird that pkg:path doesn't "do the right thing" here!

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah it is curious. I think the issue is that Isolate.resolvePackageUri gives a Uri like file:///C:/one/two, which maybe isn't incorrect because URIs should always have /, even on Windows? And then I ask for the .path out of that, which gives /C:/one/two and now we're in trouble.

@srawlins
Copy link
Member Author

srawlins commented Jun 3, 2021

I'm landing this before it breaks again.

@srawlins srawlins merged commit e58ae4e into dart-lang:master Jun 3, 2021
@srawlins srawlins deleted the aot-compiler-2 branch June 3, 2021 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Google CLA check succeeded.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants