dart-lang / test

A library for writing unit tests in Dart.
https://pub.dev/packages/test
490 stars 213 forks source link

Pass dart defines when -c kernel #2272

Open davidmorgan opened 2 weeks ago

davidmorgan commented 2 weeks ago

Testing code that uses bool.fromEnvironment, it looks like

dart -Dfoo=true test

never picks up the true value with the default -c kernel, but with -c source it does.

This took me a while to figure out, maybe it could be supported?

It looks like "flutter test" does support it via --dart-define, although I didn't try it.

Maybe just a note in the docs that it works with flutter test but there's no full equivalent in dart test?

Thanks :)

jakemac53 commented 2 weeks ago

I think we probably would want to support it like dart test -Dfoo=true - what you are doing is actually setting the define for the test runner itself, which happens to work in "source" mode because I think we run a sub-isolate and let the VM do its own compilation which inherits the defines.

jakemac53 commented 2 weeks ago

cc @natebosch this could be one answer to people asking for command line arguments to tests.

natebosch commented 2 weeks ago

I don't think we have any way to read these values within the Dart code of the test runner. This would require a change in the Dart CLI to specially forward these to the test runner. @bkonyi does that seem feasible?

jakemac53 commented 2 weeks ago

We should be able to support passing these as our own args though right? I am suggesting we add a --define (with -D shorthand) multi option argument to package:test. And pass that along to the compilers. We don't actually want the defines available to the test runner itself, just passed when compiling the tests.

Although, I don't think we could support it if running from source the way we currently do.

bkonyi commented 2 weeks ago

We want the actual test to have access to this code, right? If so, it needs to be provided at kernel compile time so package:test will need to handle that when running from kernel.

However, if we're running from source these defines need to be set at runtime (e.g., what dart -Dfoo=bar test currently does). Without support for setting environment via Isolate.spawnUri (the proposal I made in https://github.com/dart-lang/sdk/issues/55812 that was ultimately shot down), we'll need special plumbing to make this work.

jakemac53 commented 2 weeks ago

Right, I don't see how we could support this uniformly for all modes (running via Isolate.spawnUri as well as separately compiled and executed programs), because with the tools currently available the argument would have to be passed in a different location for each.

I think @natebosch was suggesting though that if the Dart CLI forwarded the -D flags that precede the test command to the test runner as regular arguments, it could be supported. Or, we would need a Dart API for the test runner to read all the -D arguments it was executed with, which I don't think is available. You can only read known variables.

This also would only work for the dart test command and not dart run test or dart bin/test.dart etc. So, still not really a full solution but would work for the majority of use cases.