Skip to content

Conversation

@jonahwilliams
Copy link
Contributor

@jonahwilliams jonahwilliams commented Nov 30, 2020

A first pass at wiring up the frontend server in the VM platform, similar to how flutter test works for compilation on the fly.

with_fe_server

@google-cla google-cla bot added the cla: yes label Nov 30, 2020
@jonahwilliams
Copy link
Contributor Author

Updated to use the frontend_server_client package. Unfortunately that doesn't yet have the --no-print-incremental-dependencies option so this slows down slightly, but not a huge deal. Someone should do the math to figure out when that is safe to use :)

@jonahwilliams
Copy link
Contributor Author

I believe the current issue is that the test_on_test.dart (and possibly others). Generate a dart file without a package config, so the compiler cannot resolve anything. This works from source since test will use the package config of the current isolate.

@jonahwilliams
Copy link
Contributor Author

Looks like I need a new version of the frontend_server client package, but otherwise I think I can get the rest going...

@jakemac53 jakemac53 marked this pull request as ready for review March 22, 2021 18:14
@jakemac53 jakemac53 requested a review from natebosch March 22, 2021 18:14
@jakemac53
Copy link
Contributor

Added you @natebosch to get another set of eyes, probably not appropriate for me to LGTM at this point given I have touched a fair bit of it as well lol

Copy link
Member

@grouma grouma left a comment

Choose a reason for hiding this comment

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

LGTM besides one question.

@jakemac53
Copy link
Contributor

There is a bit of an edgecase that still exists it looks like if you ctrl+c while the compiler is running. I am going to clean that up. I am not sure how we can consistently test that, but I am going to think on it and also manually verify the behavior.

@jakemac53 jakemac53 merged commit 55e7027 into dart-lang:master Mar 23, 2021
auto-submit bot pushed a commit to flutter/engine that referenced this pull request Sep 30, 2024
…#55475)

Closes flutter/flutter#155769.

This is a variant of the approach in #52241, based on feedback from @jakemac53 and @jonahwilliams (who originally sped up `dart test` significantly by using `frontend_server` under the scenes: dart-lang/test#1399), in short:

```gn
# tools/engine_tool/BUILD.gn

import("//flutter/build/dart/internal/gen_dartcli_call.gni")

gen_dartcli_call("tests") {
  args = [ "test" ]
  cwd = "//flutter/tools/engine_tool"
}
```

This stanza, when built (`ninja -C ../out/host_debug flutter/tools/engine_tool:tests`) generates the following file:

```sh
# ../out/host_debug/gen/flutter/tools/engine_tool/tests

set -e

# Needed because if it is set, cd may print the path it changed to.
unset CDPATH

# Store the current working directory.
prev_cwd=$(pwd)

# Set a trap to restore the working directory.
trap 'cd "$prev_cwd"' EXIT

CD_PATH="/Users/matanl/Developer/engine/src/flutter/tools/engine_tool"
if [ -n "$CD_PATH" ]; then
  cd "$CD_PATH"
fi

/Users/matanl/Developer/engine/src/flutter/prebuilts/macos-arm64/dart-sdk/bin/dart "test"
```

In turn, when executed (`../out/host_debug/gen/flutter/tools/engine_tool/tests`) it just runs `dart test`:

```sh
flutter % ../out/host_debug/gen/flutter/tools/engine_tool/tests
Building package executable... 
Built test:test.
00:00 +0: test/test_command_test.dart: test command executes test                                                                                                                                                                                                          
00:00 +3: test/run_command_test.dart: run command invokes flutter run
...
00:00 +117: All tests passed!
```

There is no actual compilation performed by the tool (that is handled implicitly by `dart test`), and as a result neither a `depfile` is needed, nor generating a pre-compiled artifact like a snapshot or AOT elf/assembly. 

---

This work is incomplete, that is, we'd want to properly tag the executable so `et` can find it, and create a wrapper template (i.e. `dart_test`) that tightens things up a bit, but I wanted to show the work at this intermediate step to get feedback before moving forward.

/cc @jonahwilliams, @jtmcdole as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants