Skip to content

dil file is being created when I run a test #4243

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

Closed
kenzieschmoll opened this issue Nov 3, 2022 · 5 comments · Fixed by flutter/flutter#115075
Closed

dil file is being created when I run a test #4243

kenzieschmoll opened this issue Nov 3, 2022 · 5 comments · Fixed by flutter/flutter#115075
Labels
in flutter Relates to running Flutter apps in testing Relates to test execution of Dart/Flutter tests for end users is bug relies on sdk changes Something that requires changes in the Dart/Flutter SDK to ship before it will become available
Milestone

Comments

@kenzieschmoll
Copy link

When I run a test from VS Code (by selecting the "Debug" option that floats above the tests main method), VS code creates a dil file with the same name as the test. I have to manually delete these files from source control before committing.

Screen Shot 2022-11-03 at 9 47 30 AM

@DanTup
Copy link
Member

DanTup commented Nov 3, 2022

I can repro outside of VS Code by running from the terminal:

flutter test --start-paused test/shared/table_test.dart

It does not seem to occur if you don't pass --start-paused though. This may be related to a change I made in Flutter recently - flutter/flutter#113481 - to support expression evaluation in integration tests. I think it might be triggering a compile that it did not before (and without a path, it's just putting it alongside the test file).

I'll take a look.

@DanTup DanTup added this to the v3.54.0 milestone Nov 3, 2022
@DanTup DanTup added is bug in flutter Relates to running Flutter apps in testing Relates to test execution of Dart/Flutter tests for end users labels Nov 3, 2022
@DanTup
Copy link
Member

DanTup commented Nov 3, 2022

@christopherfujino I've had a look at this and now understand the issue, but I'm not sure how best to fix.

The issue is that in flutter/flutter#113481 I made a change that results in the test file being compiled for unit tests where it was not previously:

        final Uri testUri = globals.fs.file(testPath).uri;
        // Trigger a compilation to initialize the resident compiler.
        unawaited(compiler!.compile(testUri));

I can easily fix this and avoid that code running for unit tests (because it's unnecessary) and the problem will go away. However, the same problem exists for integration tests, because in order to initialize the compiler we're still running the code above for those.

For unit tests, the thing we compile to set up the expression compiler is a "listener" file in the temporary directory (so the dill is created alongside that). But for integration tests, there's no such file (at least not in flutter_platform.dart) so we don't have a temporary file to compile (and if we compile the test file we leave a dill next to it, and if we instead just always run the code that generates the listener file and compile that, it seems to hang in the case of integration tests).

One fix would be to write an empty/dummy temporary file and compile that, but it feels like a hack. I'm not familiar enough with this code to know if there's a better way.

@christopherfujino
Copy link

@christopherfujino I've had a look at this and now understand the issue, but I'm not sure how best to fix.

The issue is that in flutter/flutter#113481 I made a change that results in the test file being compiled for unit tests where it was not previously:

        final Uri testUri = globals.fs.file(testPath).uri;
        // Trigger a compilation to initialize the resident compiler.
        unawaited(compiler!.compile(testUri));

I can easily fix this and avoid that code running for unit tests (because it's unnecessary) and the problem will go away. However, the same problem exists for integration tests, because in order to initialize the compiler we're still running the code above for those.

For unit tests, the thing we compile to set up the expression compiler is a "listener" file in the temporary directory (so the dill is created alongside that). But for integration tests, there's no such file (at least not in flutter_platform.dart) so we don't have a temporary file to compile (and if we compile the test file we leave a dill next to it, and if we instead just always run the code that generates the listener file and compile that, it seems to hang in the case of integration tests).

One fix would be to write an empty/dummy temporary file and compile that, but it feels like a hack. I'm not familiar enough with this code to know if there's a better way.

chatted offline with Danny, and from reading the code it's not obvious to me why we're creating dill next to the source file, as it looks like we should be compiling to a temp file that gets cleaned up afterwards. I believe Danny will investigate further.

@DanTup
Copy link
Member

DanTup commented Nov 8, 2022

@christopherfujino so, I figured out it was this line causing the copy:

https://github.com/flutter/flutter/blob/a1432a9c5da2f2fef9241d9f6d3f6b3a69bc918b/packages/flutter_tools/lib/src/test/test_compiler.dart#L189

I don't really know what the purpose of this copy is, since it's just a copy of another file. I tried removing the copy here:

flutter/flutter@eddf9d8

However lots of tests broke with non-obvious reasons:

https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8798064336178227617/+/u/run_test.dart_for_framework_tests_shard_and_subshard_slow/test_stdout

Failed to load "/b/s/w/ir/x/w/flutter/dev/tracing_tests/test/inflate_widget_tracing_test.dart": Shell subprocess crashed with unexpected exit code -7.

With the original code, in the case of running tests here the file we compiled as a temporary file so dumping the dill next to it was fine. After my changes, I was compiling the test file which meant a dill was produced there.

So, I'm trying to use an empty temporary file instead (for integration tests - with unit tests reverting to their original behaviour of compiling the "dart listener file"):

flutter/flutter@5cbab15

This change is mostly just reverting the original change, but now produces an empty mainDart in the case of integration tests:

mainDart = integrationTestDevice == null
            ? _createListenerDart(finalizers, ourTestCount, testPath)
            : _createExpressionEvaluationDart(finalizers, ourTestCount);

I don't think this is a good solution, but it will be good to understand if it works. There's actually a slight change in behaviour here as mainDart used to be the listener file (even though it was not compiled) and now it's an empty file, so it may still fail. If it does, I'm going to try going back to always producing the standard mainDart (the listener file) and see if I can figure out why trying to compile it (which works for unit tests) fails for integration tests (it seems odd to compile it if it's not used, but it seems like we need to compile something and perhaps an existing file we're already using in some other cases makes more sense than an empty temp file?).

(I'll do some more digging after lunch when bots on flutter/flutter#114887 will hopefully have completed)

@DanTup
Copy link
Member

DanTup commented Nov 8, 2022

@christopherfujino I may require some assistance 😄. All of the things I tried were unsuccessful..

  • Creating an "empty" dummy file causes test failures complaining about sound-null-safety mismatching (even if I used the same language header added to the mainDart file)
  • Just compiling mainDart in all cases (instead of only for unit tests) just hangs and the compiler emits a large about of errors as if it's trying to compile a binary file as Dart code (it prints a lot of non-ascii characters and compile errors)
  • Using the outputDill variable that was configured in the TestCompiler constructor instead of the one being copied to in the code I linked above causes [other failures](https://ci.chromium.org/ui/p/flutter/builders/try/Windows%20tool_tests_commands/17900/overview - "You've created a plugin project that doesn't yet support any platforms") that I wonder if may be caused by that file no longer being inside the project

Perhaps part of the problem is that I don't understand why there are so many files in this code here:

https://github.com/flutter/flutter/blob/a1432a9c5da2f2fef9241d9f6d3f6b3a69bc918b/packages/flutter_tools/lib/src/test/test_compiler.dart#L187

  • testFile
  • mainUri
  • kernelReadyToRun
  • testCache
  • outputFile
  • and on the class, outputDill

Many of these files are just copies of others, and some of them don't even seem to be used directly (for example testCache is a file that's checked for existence, but then only used to create its parent directory but not actually used?).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in flutter Relates to running Flutter apps in testing Relates to test execution of Dart/Flutter tests for end users is bug relies on sdk changes Something that requires changes in the Dart/Flutter SDK to ship before it will become available
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants