-
Notifications
You must be signed in to change notification settings - Fork 215
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 support for specifying a compiler separately from a platform #1776
Comments
This will require a change in the interface between the test runner, and each
cc @christopherfujino - heads up that this will impact integration with flutter_test, likely sometime in Q1. Does the flutter test setup have any notion of running with different compilation approaches on the same platform today? |
- Adds the experimental platform `experimental-chrome-wasm`. This requires chrome beta and a very recent SDK to run. - This platform will be deleted eventually, and replaced with a [--compiler](#1776) flag. - Adds `wasm` to the boolean selector variables, and an `isWasm` field to `Runtime` (defaults to false, so should be non-breaking). Note that the integration test for this will initially be failing, as there is one more change needed from the SDK, but once that lands we can update this to point at the new location for the wasm runtime, update the min SDK, re-run the tests, and land.
I have been looking into this a bit, there are a few questions we will want to resolve that I have ran into:
|
Another question, should we add a special |
I think it makes sense to avoid doing any compilation ourselves in precompiled mode. We should disallow I'm not sure if that introduces difficulty with the browser platforms - would they need to set things up different based on detecting a precompiled wasm file over a precomiled js file?
This seems sensible to me, and I like that it sidesteps the other problem below.
I think it's specifically breaking to add support for an existing compiler to an existing platform. So we'd need to know whenever adding a compiler which existing platforms will ever support it, and when adding a platform which existing compilers it will ever support. We can add brand new compilers supported by existing platforms, and we can add new platforms which support existing compilers. I think the biggest risk there is the strangeness with flutter already using DDC and
Yeah, I think we should leave it up to trust. If you told us that you handled compilation, we just run the platform and hope that you did indeed compile to something supported by the platform.
Dropping this flag in favor of
Brainstorming: You mentioned the idea of using |
Right, I want to also add AOT support to the VM platform. Both AOT and wasm are examples of things that would have broken us, as they are both new compilers that were added to known platforms.
SGTM
It is more than just this - you could have pre-compiled files for multiple compilers present in the precompiled dir. So I think we want to keep the In other words, I would expect
I do in general like the idea of something like this, I can't decide if I like |
I don't think that use case would break. New compilers on known platforms should be fine - no existing scripts would have a working In any case, it's likely moot since I think we're moving in the direction of using boolean selector to more flexibly choose compilers and cover the situation of using
I'm not sure that I agree. It seems like I'm ok with treating the flag as a filter in precompiled mode for the reasons you state. Will we plan on having some kind of warning if you pass |
But we have to know which precompiled files to run. These aren't just old artifacts they might all be valid for the current run.
That is an interesting case because we expect the same JS files :(. We could potentially change it though, to expect different entrypoint JS files per compiler? But we don't have any build system today that would support compiling the same app with two different compilers and running those both via a single test. I am not sure if we want to actually hard code a warning or not. |
If we did come up with a standard for using a different extension for DDC tests we could probably support it without too much work. I still think it would be nice in general to have separate intermediate files between dart2js and ddc and the copy the entrypoint as the last step, so that we avoid full recompile when switching compilers. |
Only dart2js actually ends up having to recompile, but this is fair. I worry a bit about just copying it due to the potential size of dart2js files though. Maybe what I would do if I could go back in time (or do a breaking change?) would be to have people use some sort of magic template identifier to load the JS, instead of hardcoding a |
We should also consider if we should add an |
Closes #1776 ## Overview - Adds a user facing `--compiler` flag, which can be used to configure which compiler is used to run tests. - Adds a new `Compiler` class, similar to the `Runtime` class, but it doesn't support custom ones (at least for now). - This is exported from `package:test_api/backend.dart` - Adds required `defaultCompiler` and `supportedCompilers` fields to `Runtime`. This is breaking for custom runtimes. - Platforms will not be asked to load tests for compilers that the associated runtimes don't support. - Add `compiler` field to `SuitePlatform`. This is how platforms get access to compiler configuration. It means there is no API breaking change for platforms, but they do need to be updated to support the configuration. - Adds support in all platforms to respect the compiler option. - The vm platform supports the `source` compiler which is roughly equivalent to `--use-data-isolate-strategy`. The `--use-data-isolate-strategy` flag is now just an alias for `--compiler source` and it it deprecated/hidden. - We would call this compiler `none` but we already have `OS.none`, so it would conflict in boolean selectors. - Adds a `CompilerSelection` class similar to `RuntimeSelection`, used by `SuiteConfig` to track the selected compiler(s). - Adds a new `compilerSelections` field to `SuiteConfig`. - Adds the compilers to the valid boolean selectors, this makes them usable in `TestOn` etc.
As we look to add support for compiling to wasm and running the browser, we should add a
--compiler
flag that allows specifying one or more compilers to use in combination with the specified platforms. Using a separate flag will allow using the wasm compiler with any browser that supports the required wasm features.We'll add a
wasm
term similar to thejs
term, for use in@TestOn
configuration.When no
--compiler
flag is passed, each platform will have a default compiler. When a--compiler
flag is passed, only the valid combinations of compiler ✕ platform will run. So a combination like--compiler dart2js --platform vm
will run no tests, since the VM platform does not support dart2js as a compiler, but--compiler jit,dart2js,wasm --platform vm,chrome
would run both dart2js and wasm tests on chrome, as well as from-source tests on the VM.In the future, we should be able to add support for compiling with DDC (monolithic compile with the frontend server, it's unlikely we'd implement modular compiles), and may consider adding support for AOT compiles on the VM platform.
The text was updated successfully, but these errors were encountered: