dart-lang / test

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

Various errors on build_runner + node #862

Open lexaknyazev opened 6 years ago

lexaknyazev commented 6 years ago
Dart VM version: 2.0.0-dev.60.0 (Mon Jun 4 22:09:13 2018 +0200) on "windows_x64"
import 'package:test/test.dart';
void main() {
  test('True test', () {
    expect(true, isTrue);
  });
}

Compiles and runs fine with -p chrome.

> pub run build_runner test -- -p chrome
[INFO] Generating build script completed, took 697ms
[INFO] Reading cached asset graph completed, took 308ms
[INFO] Checking for updates since last build completed, took 486ms
[INFO] Running build completed, took 192ms
[INFO] Caching finalized dependency graph completed, took 131ms
[INFO] Creating merged output dir `C:\Users\alexey\AppData\Local\Temp\build_runner_test5e375219-6c0f-11e8-85b0-1c1b0d0d6f72\` completed, took 12.9s
[INFO] Writing asset manifest completed, took 10ms
[INFO] Succeeded after 13.3s with 0 outputs (0 actions)
Running tests...

00:01 +1: All tests passed!

Fails to compile with -p node.

> pub run build_runner test -- -p node
[INFO] Generating build script completed, took 685ms
[INFO] Reading cached asset graph completed, took 302ms
[INFO] Checking for updates since last build completed, took 470ms
[INFO] Running build completed, took 200ms
[INFO] Caching finalized dependency graph completed, took 131ms
[INFO] Creating merged output dir `C:\Users\alexey\AppData\Local\Temp\build_runner_test9bd8b120-6c0f-11e8-85b0-1c1b0d0d6f72\` completed, took 12.7s
[INFO] Writing asset manifest completed, took 8ms
[INFO] Succeeded after 13.1s with 0 outputs (0 actions)
Running tests...

00:00 +0 -1: compiling test\validate_test.dart [E]
  Failed to load "test\validate_test.dart":
  Cannot open file, path = 'C:\Users\alexey\AppData\Local\Temp\build_runner_test9bd8b120-6c0f-11e8-85b0-1c1b0d0d6f72\test\validate_test.dart.node_test.dart.js.map' (OS Error: The system cannot find the file specified., errno = 2)
00:00 +0 -1: Some tests failed.

Fails inside node with -p node --js-trace.

> pub run build_runner test -- -p node --js-trace
[INFO] Generating build script completed, took 690ms
[INFO] Reading cached asset graph completed, took 351ms
[INFO] Checking for updates since last build completed, took 492ms
[INFO] Running build completed, took 215ms
[INFO] Caching finalized dependency graph completed, took 132ms
[INFO] Creating merged output dir `C:\Users\alexey\AppData\Local\Temp\build_runner_testf54c97c4-6c0f-11e8-85b0-1c1b0d0d6f72\` completed, took 12.9s
[INFO] Writing asset manifest completed, took 14ms
[INFO] Succeeded after 13.3s with 0 outputs (0 actions)
Running tests...

00:00 +0: compiling test\validate_test.dart
internal/modules/cjs/loader.js:596
    throw err;
    ^

Error: Cannot find module 'C:\Users\alexey\AppData\Local\Temp\build_runner_testf54c97c4-6c0f-11e8-85b0-1c1b0d0d6f72\test\validate_test.dart.node_test.dart.js'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:594:15)
    at Function.Module._load (internal/modules/cjs/loader.js:520:25)
    at Function.Module.runMain (internal/modules/cjs/loader.js:744:10)
    at startup (internal/bootstrap/node.js:238:19)
    at bootstrapNodeJSCore (internal/bootstrap/node.js:572:3)
lexaknyazev commented 6 years ago

More info.

The tests above have been run with

dev_dependencies:
  build_runner: ^0.8.2
  build_test:
  build_web_compilers: '>=0.3.6 <0.5.0'
  test: ^0.12.40

Adding build_node_compilers results in a different crash:

dev_dependencies:
  build_runner: ^0.8.2
  build_test:
  build_web_compilers: '>=0.3.6 <0.5.0'
  build_node_compilers:
  test: ^0.12.40
> pub run build_runner test -- -p node
[INFO] Generating build script completed, took 641ms
[WARNING] Throwing away cached asset graph because the build phases have changed. This most commonly would happen as a result of adding a new dependency or updating your dependencies.
[INFO] Cleaning up outputs from previous builds. completed, took 203ms
[INFO] Reading cached asset graph completed, took 481ms
[INFO] Building new asset graph...

You have hit a bug in build_runner
Please file an issue with reproduction steps at https://github.com/dart-lang/build/issues

DuplicateAssetNodeError: GeneratedAssetNode: bug_lib|test/validate_test.dart.bootstrap.js generated from input bug_lib|test/validate_test.dart.
This probably means you have multiple actions that are trying to output the same file.
package:build_runner/src/asset_graph/graph.dart 468:11          AssetGraph._addGeneratedOutputs
package:build_runner/src/asset_graph/graph.dart 420:21          AssetGraph._addInBuildPhaseOutputs
package:build_runner/src/asset_graph/graph.dart 386:26          AssetGraph._addOutputsForSources
package:build_runner/src/asset_graph/graph.dart 55:11           AssetGraph.build
package:build_runner/src/generate/build_definition.dart 104:39  _Loader.prepareWorkspace.<fn>
package:build_runner/src/logging/logging.dart 25:30             logTimedAsync
package:build_runner/src/generate/build_definition.dart 103:13  _Loader.prepareWorkspace
package:build_runner/src/generate/build_definition.dart 60:54   BuildDefinition.prepareWorkspace
package:build_runner/src/generate/build_impl.dart 115:29        _singleBuild
package:build_runner/src/generate/build_impl.dart 104:20        build
package:build_runner/src/generate/build.dart 71:5               build
package:build_runner/src/entrypoint/test.dart 82:26             TestCommand.run
package:args/command_runner.dart 194:27                         CommandRunner.runCommand
package:args/command_runner.dart 109:29                         CommandRunner.run.<fn>
dart:async                                                      new Future.sync
package:args/command_runner.dart 109:11                         CommandRunner.run
package:build_runner/src/entrypoint/run.dart 22:31              run
.dart_tool\build\entrypoint\build.dart 81:22                    main

Replacing build_web_compilers with build_node_compilers apparently fixes the issue

dev_dependencies:
  build_runner: ^0.8.2
  build_test:
  build_node_compilers:
  test: ^0.12.40
> pub run build_runner test -- -p node
[INFO] Generating build script completed, took 573ms
[WARNING] Throwing away cached asset graph because the build phases have changed. This most commonly would happen as a result of adding a new dependency or updating your dependencies.
[INFO] Cleaning up outputs from previous builds. completed, took 9ms
[INFO] Reading cached asset graph completed, took 294ms
[INFO] Building new asset graph completed, took 704ms
[INFO] Checking for unexpected pre-existing outputs. completed, took 4ms
[INFO] Running build completed, took 19.7s
[INFO] Caching finalized dependency graph completed, took 99ms
[INFO] Creating merged output dir `C:\Users\alexey\AppData\Local\Temp\build_runner_testa75ee03b-6c8d-11e8-85b0-1c1b0d0d6f72\` completed, took 11.2s
[INFO] Writing asset manifest completed, took 8ms
[INFO] Succeeded after 31.3s with 285 outputs (411 actions)
[INFO] BuildModules:Failed to delete temp dir C:\Users\alexey\AppData\Local\Temp\scratch_spaceb00b6b21-6c8d-11e8-85b0-1c1b0d0d6f72, retrying in 100msRunning tests...

00:00 +1: All tests passed!
jakemac53 commented 6 years ago

Yes - if running on node then you need to use build_node_compilers and not build_web_compilers.

If you want to run both in the same project, you will need to configure them with generate_for so they don't try to compile the same tests, something like this:

targets:
  $default:
    builders:
      build_web_compilers|entrypoint:
        generate_for:
          - web/**.dart
          - test/**.vm_test.dart
      build_node_compilers|entrypoint:
        generate_for:
          - node/**.dart
          - test/**.node_test.dart
lexaknyazev commented 6 years ago

In my case, tests are exactly the same for both runtimes so it would be nice for build_runner to handle that source duplication automatically. There's even a selector @TestOn('js') which seems to cover both Chrome and Node.

matanlurey commented 6 years ago

@TestOn() has nothing to do with build_runner.

(FWIW, I think we might want to re-think these tags in the Dart2+ world @jakemac53 @grouma)

jakemac53 commented 6 years ago

In my case, tests are exactly the same for both runtimes so it would be nice for build_runner to handle that source duplication automatically.

The builders themselves need to handle this - note that the only reason it doesn't work out of the box today is that the default generate_for globs for both of these builders include test/**/*.dart. For build_web_compilers this is actually meaningful because it allows you to debug the test in a browser directly without going through the test runner which doesn't have good debugging support today.

I don't know if the same holds for build_node_compilers though so we may just want to remove that glob and only run by default on test/**/*.node_test.dart? cc @pulyaevskiy for input there.

There's even a selector @TestOn('js') which seems to cover both Chrome and Node.

As matan mentioned only package:test knows about this annotation today - we may eventually do smarter things here but it wouldn't actually solve this issue.

lexaknyazev commented 6 years ago

I thought that there could be (at some point) a "test_builder" that takes _test.dart files and produces _node_test.dart and _web_test.dart depending on those package:test selectors.

jakemac53 commented 6 years ago

I thought that there could be (at some point) a "test_builder" that takes _test.dart files and produces _node_test.dart and _web_test.dart depending on those package:test selectors.

Yes - this is already a builder but it doesn't read annotations right now. Reading the annotations is actually pretty expensive because it requires resolving the world, but we may eventually do it or do a sort of dumber but faster version where we only parse the file.

jakemac53 commented 6 years ago

Note though that the issue here isn't that those files always get created - its that both packages are trying to compile the original test.

lexaknyazev commented 6 years ago

In addition to that, in the first failure case, package:test tries to run tests compiled with build_web_compilers on node. Probably, build_runner and test should signal something to each other to filter out such combinations.

jakemac53 commented 6 years ago

In addition to that, in the first failure case, package:test tries to run tests compiled with build_web_compilers on node.

You actually got a load error here - the issue was the node tests weren't compiled at all. It didn't try to run them with the web compiled versions.

Failed to load "test\validate_test.dart":
  Cannot open file, path = 'C:\Users\alexey\AppData\Local\Temp\build_runner_test9bd8b120-6c0f-11e8-85b0-1c1b0d0d6f72\test\validate_test.dart.node_test.dart.js.map' (OS Error: The system cannot find the file specified., errno = 2)

It then continued to actually try to run the test it appears, which just failed to load the node module:

Error: Cannot find module 'C:\Users\alexey\AppData\Local\Temp\build_runner_testf54c97c4-6c0f-11e8-85b0-1c1b0d0d6f72\test\validate_test.dart.node_test.dart.js'

The error reporting here could definitely be improved though.

grouma commented 6 years ago

We should open a separate tracking bug to improve the error or not run the test after failing to load it.