Support for node platform with --precompiled flag

[pulyaevskiy]

I was trying to get build_runner to build and run tests in node_interop package but pub run test --precompiled doesn't seem to support node platform yet. It always falls back to compile the source itself.

I have created build_node_compilers package which produces Node-compatible output. If I just build my tests and run them manually (e.g. node build/test/some_test.dart.js) it works fine. But it'd be really great to have it running through the standard tooling.

[nex3]

--precompiled is not an official part of the test runner's API. It was added for Google-internal tooling, and any use of it outside of that context is unsupported. As such, I don't want to expand the set of things it happens to support and thereby encourage more people to use it.

That said, I'm definitely interested in getting the full matrix of build/DDC/platform configurations, and I have some experimental work in progress to that end that will hopefully come out on the sooner end.

[pulyaevskiy]

That makes sense, thanks for quick response. If there is anywhere I can click "Subscribe" to follow progress on this front please let me know!

[nex3]

I'll try to remember to post something here once code reviews start going up.

Out of curiosity, are you interested in build mostly for the speed improvements of DDC or because you're running tests against generated code?

[pulyaevskiy]

Mostly to validate that my JS bindings work in both DDC and dart2js (as well as Dart code using these bindings). There are differences in how JS interop is interpreted by both compilers, the same code can run perfectly fine in dart2js but fail badly in DDC and vice versa.

So I needed a way to run tests with both of them. For now I'm just gonna use a little shell script as a workaround:

#!/bin/sh

pub run build_runner build --output=build/

for filename in test/*_test.dart; do
    node "build/$filename.js"
done

[jakemac53]

I am going to see if I can tackle this - we are changing the semantics and support level for --precompiled so it can be used reliably with build_runner for all scenarios.

[jakemac53]

@pulyaevskiy this is published now if you want to give it a shot! Please file issues if it doesn't work for you :).

[jakemac53]

hmm I checked out out the node_interop repo and I am seeing a couple problems, sent out #807 which resolves the first issue.

[pulyaevskiy]

Thanks @jakemac53 ! This is great. Just tried a quick run with new versions and it almost works. I'm getting errors like:

compiling test/console_test.dart [E]
  Failed to load "test/console_test.dart": Cannot open file, path = 'build/console_test.dart.node_test.dart.js.map' (OS Error: No such file or directory, errno = 2)

I tried to run test with --precompiled build/test/ but then it couldn't find .packages file.

Might need to fix something in build_node_compilers, not sure yet.

Update: Just saw your PR, this might fix it.

[jakemac53]

Yep - that issue should be fixed with my latest pr, can you add a dependency override on that branch and try?

Should look roughly like this:

dependency_overrides:
  test:
    git:
      url: https://github.com/dart-lang/test
      ref: node-fixes

[pulyaevskiy]

Sure, checking it now

[jakemac53]

I am getting a weird permissions issue with Node locally - not sure what exactly the problem is though so I am curious if you get the same thing.

[pulyaevskiy]

It looks better now:

Cannot open file, path = 'build/test/util_test.dart.node_test.dart.js.map' (OS Error: No such file or directory, errno = 2)

This maybe something I should fix in build_node_compilers? Probably filenames don't match?

[jakemac53]

You probably just need to add a dependency on build_test (you didn't need that before because you weren't using the test runner)

[pulyaevskiy]

Yeah, added build_test and the error went away.

Now I'm getting some DDC/strong mode warnings with streams. And for dart2js it ran successfully except it only executed 1 test file (buffer_test) of total 8 in test/ folder.

[jakemac53]

Ok - getting somewhere! I think the strong mode errors are legit errors that need to be fixed in the node test runner bootstrapping code.

[pulyaevskiy]

Oh, I take it back. Looks like all tests ran it's just output is somewhat weird

[jakemac53]

What was weird about the output?

[pulyaevskiy]

If I use -r expanded each run prints results in different order and always skipping some of the tests. E.g.:

00:00 +0: compiling test/console_test.dart
00:00 +0: test/child_process_test.dart: child_process exec successful
00:00 +1: test/child_process_test.dart: child_process exec successful
00:00 +2: test/child_process_test.dart: child_process exec successful
00:00 +3: test/timers_test.dart: Timers setImmediate
00:00 +4: test/buffer_test.dart: Buffer from array
00:00 +5: test/console_test.dart: console integration
00:00 +5: test/buffer_test.dart: Buffer keys()
  Skip: Broken in dart2js
00:00 +5 ~1: test/console_test.dart: console integration
00:00 +5 ~1: test/buffer_test.dart: Buffer values()
  Skip: Broken in dart2js
00:00 +5 ~2: test/console_test.dart: console integration
00:00 +5 ~2: test/buffer_test.dart: Buffer entries()
  Skip: Broken in dart2js
00:00 +5 ~3: test/console_test.dart: console integration
00:00 +6 ~3: test/promises_test.dart: promiseToFuture
00:00 +7 ~3: test/util_test.dart: dartify it handles js primitives
00:00 +8 ~3: test/util_test.dart: dartify it handles js primitives
00:00 +9 ~3: test/util_test.dart: dartify it handles js primitives
00:00 +10 ~3: test/util_test.dart: dartify it handles js primitives
00:00 +11 ~3: test/promises_test.dart: reject a Future
00:00 +12 ~3: test/util_test.dart: dartify it handles POJOs
00:00 +13 ~3: test/util_test.dart: dartify it handles arrays
00:00 +14 ~3: test/util_test.dart: dartify it DOES handle JS functions with properties
00:00 +15 ~3: test/util_test.dart: dartify it handles objects with prototypes
00:00 +16 ~3: All tests passed!

Here it's missing stream_test.dart, for instance.

[pulyaevskiy]

I'm sure it runs all the tests as numbers in the end match, so it's probably something unrelated. I think some time ago I saw similar issues with regular tests in Dart VM and it had something to do with how test package runs tests in parallel.

Update: this reminded me of -j flag. Running with -j 1 shows all the tests, so we should be good.

[jakemac53]

I'm sure it runs all the tests as numbers in the end match, so it's probably something unrelated. I think some time ago I saw similar issues with regular tests in Dart VM and it had something to do with how test package runs tests in parallel.

Ya I think you are right, the order I think is intentionally not consistent but the fact that the names are wrong is pretty weird.

[jakemac53]

Ok, merged that PR but going to try and fix the runtime error for DDC before I publish a new version.

[jakemac53]

dart-archive/stream_channel#18 was one of the issues, was quite difficult to track down lol

[jakemac53]

@pulyaevskiy can you try now with a dependency override on master? You will also need to pub upgrade to get the latest stream_channel.

[pulyaevskiy]

Checking this now. Will update in a few.

[pulyaevskiy]

Everything works great as far as I can tell. I ran tests in node_interop, node_io and node_http packages with both dartdevc and dart2js precompiled sources and there was no issues.

So cool!

One question: I noticed in #811 we drop support for Dart 1 and it's only a patch version bump on this package. Shouldn't it at least go to 0.13.0? Seems kind of like a major update, but regardless asking just out of curiosity.

[natebosch]

we drop support for Dart 1 and it's only a patch version bump on this package.

yes, the "breaking" change is already managed by pub so you can't version solve to an incompatible version of package:test.

[pulyaevskiy]

Ah, right, so Pub would only consider versions with appropriate SDK constraint when solving? Makes sense, thanks for explanation!

[jakemac53]

Yup! Glad to hear it works we should get this published pretty soon.

[jakemac53]

Closing as this is just waiting on a release