[go: nahoru, domu]

Skip to content
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

Some tests no longer work with custom local builds #56041

Closed
eseidel opened this issue Jun 18, 2024 · 24 comments
Closed

Some tests no longer work with custom local builds #56041

eseidel opened this issue Jun 18, 2024 · 24 comments
Labels
area-test Cross-cutting test issues (use area- labels for specific failures; not used for package:test). triage-automation Issues triaged by automation (see https://github.com/dart-lang/sdk/wiki/Triage-automation) type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@eseidel
Copy link
Contributor
eseidel commented Jun 18, 2024

Some tests no longer work with custom configurations.

These used to work back in 3.19.x (3.3.x) Dart and broke with 3.4.x. (Edit: broke at ca9eb4a)

./tools/build.py --mode release create_sdk
python3 tools/test.py -m release -c dartkp -r dart_precompiled -a arm64 --use-elf vm/dart/use_trace_precompiler_flag_test -k

And you will get:

No build targets found.
Test configuration:
    custom-configuration-1(architecture: arm64, compiler: dartkp, mode: release, runtime: dart_precompiled, system: mac)
Suites tested: vm

FAILED: dartkp-dart_precompiled release_arm64 vm/dart/use_trace_precompiler_flag_test
Expected: Pass
Actual: RuntimeError

--- Command "vm_compile_to_kernel" (took 04.000355s):
DART_CONFIGURATION=ReleaseARM64 /Users/eseidel/Documents/GitHub/unmodified-dart-sdk/sdk/pkg/vm/tool/gen_kernel --aot --platform=xcodebuild/ReleaseARM64/vm_platform_strong.dill -o /Users/eseidel/Documents/GitHub/unmodified-dart-sdk/sdk/xcodebuild/ReleaseARM64/generated_compilations/custom-configuration-1/runtime_tests_vm_dart_use_trace_precompiler_flag_test/out.dill /Users/eseidel/Documents/GitHub/unmodified-dart-sdk/sdk/runtime/tests/vm/dart/use_trace_precompiler_flag_test.dart -Dtest_runner.configuration=custom-configuration-1 --packages=/Users/eseidel/Documents/GitHub/unmodified-dart-sdk/sdk/.dart_tool/package_config.json -Ddart.vm.product=false --sound-null-safety

exit code:
0

--- Command "precompiler" (took 808ms):
DART_CONFIGURATION=ReleaseARM64 xcodebuild/ReleaseARM64/gen_snapshot --snapshot-kind=app-aot-elf --elf=/Users/eseidel/Documents/GitHub/unmodified-dart-sdk/sdk/xcodebuild/ReleaseARM64/generated_compilations/custom-configuration-1/runtime_tests_vm_dart_use_trace_precompiler_flag_test/out.aotsnapshot --loading-unit-manifest=/Users/eseidel/Documents/GitHub/unmodified-dart-sdk/sdk/xcodebuild/ReleaseARM64/generated_compilations/custom-configuration-1/runtime_tests_vm_dart_use_trace_precompiler_flag_test/ignored.json --sound-null-safety -Dtest_runner.configuration=custom-configuration-1 --ignore-unrecognized-flags --packages=/Users/eseidel/Documents/GitHub/unmodified-dart-sdk/sdk/.dart_tool/package_config.json /Users/eseidel/Documents/GitHub/unmodified-dart-sdk/sdk/xcodebuild/ReleaseARM64/generated_compilations/custom-configuration-1/runtime_tests_vm_dart_use_trace_precompiler_flag_test/out.dill

exit code:
0

--- Command "vm" (took 176ms):
DART_CONFIGURATION=ReleaseARM64 TEST_COMPILATION_DIR=/Users/eseidel/Documents/GitHub/unmodified-dart-sdk/sdk/xcodebuild/ReleaseARM64/generated_compilations/custom-configuration-1/runtime_tests_vm_dart_use_trace_precompiler_flag_test xcodebuild/ReleaseARM64/dart_precompiled_runtime --sound-null-safety -Dtest_runner.configuration=custom-configuration-1 --ignore-unrecognized-flags --packages=/Users/eseidel/Documents/GitHub/unmodified-dart-sdk/sdk/.dart_tool/package_config.json /Users/eseidel/Documents/GitHub/unmodified-dart-sdk/sdk/xcodebuild/ReleaseARM64/generated_compilations/custom-configuration-1/runtime_tests_vm_dart_use_trace_precompiler_flag_test/out.aotsnapshot

exit code:
255

stderr:
Unhandled exception:
FormatException: Must specify at least one of compiler or runtime in options or configuration name.
#0      Configuration.parse (package:smith/configuration.dart:251)
#1      _configuration (package:expect/config.dart:18)
#2      isVmAotConfiguration (package:expect/config.dart)
#3      isAOTRuntime (file:///Users/eseidel/Documents/GitHub/unmodified-dart-sdk/sdk/runtime/tests/vm/dart/use_flag_test_helper.dart:12)
#4      main (file:///Users/eseidel/Documents/GitHub/unmodified-dart-sdk/sdk/runtime/tests/vm/dart/use_trace_precompiler_flag_test.dart)
#5      _delayEntrypointInvocation.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:295)
#6      _RawReceivePort._handleMessage (dart:isolate-patch/isolate_patch.dart:184)

--- Re-run this test:
python3 tools/test.py -m release -c dartkp -r dart_precompiled -a arm64 --use-elf -k vm/dart/use_trace_precompiler_flag_test
[00:05 | 100% | +    0 | -    1]

=== 0 tests passed, 1 failed ===
@dart-github-bot
Copy link
Collaborator

Labels: area-test, type-bug
Summary: Custom configurations for tests using dartkp compiler and dart_precompiled runtime on ARM64 architecture are failing with a FormatException due to missing compiler or runtime specification in the configuration name. This issue was introduced in Dart 3.4.x and did not exist in 3.19.x or 3.3.x.

@dart-github-bot dart-github-bot added area-test Cross-cutting test issues (use area- labels for specific failures; not used for package:test). triage-automation Issues triaged by automation (see https://github.com/dart-lang/sdk/wiki/Triage-automation) type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Jun 18, 2024
@eseidel
Copy link
Contributor Author
eseidel commented Jun 18, 2024

If you do:

git checkout 3.3.4
gclient sync -D
./tools/build.py --mode release create_sdk
python3 tools/test.py -m release -c dartkp -r dart_precompiled -a arm64 --use-elf vm/dart/use_trace_precompiler_flag_test

Then the test passes just fine. :)

@eseidel
Copy link
Contributor Author
eseidel commented Jun 18, 2024

0dbeb21 is the only change to this part of test_runner in that time (from @nshahan), but it's not at all clear that that's the cause.

Presumably --named-configuration is now required? If so, I'm not sure what I'm supposed to pass.

the custom-configuration-1 appears to be generated via:

"custom-configuration-${configurationNumber++}",

Which is only hit if namedConfigurations (earlier in the file) is empty.

Maybe my test.py line is just an old style? https://github.com/dart-lang/sdk/blob/main/docs/Testing-the-VM.md doesn't seem to cover this case.

@eseidel
Copy link
Contributor Author
eseidel commented Jun 18, 2024

Example vm tests which fail this way:

vm/dart/build_id_test : RuntimeError
vm/dart/no_local_labels_test : RuntimeError
vm/dart/ama_test : RuntimeError
vm/dart/print_object_layout_test : RuntimeError
vm/dart/regress_54871_test : RuntimeError
vm/dart/use_strip_flag_test : RuntimeError
vm/dart/use_add_readonly_data_symbols_flag_test : RuntimeError
vm/dart/spawn_uri_aot_test : RuntimeError
vm/dart/unobfuscated_static_symbols_test : RuntimeError
vm/dart/exported_symbols_test : RuntimeError
vm/dart/use_save_debugging_info_flag_test : RuntimeError
vm/dart/isolates/regress_54528_test : RuntimeError
vm/dart/isolates/reload_many_isolates_test : RuntimeError
vm/dart/isolates/reload_active_stack_test : RuntimeError
vm/dart/isolates/regress_46539_test : RuntimeError
vm/dart/isolates/reload_many_isolates_live_and_die_test : RuntimeError
vm/dart/isolates/reload_no_active_stack_test : RuntimeError
vm/dart/disassemble_aot_test : RuntimeError
vm/dart/regress_48523_test : RuntimeError
vm/dart/gen_snapshot_include_resolved_urls_test : RuntimeError
vm/dart/entrypoints/aot/static_this_test : RuntimeError
vm/dart/entrypoints/aot/super_test : RuntimeError
vm/dart/use_code_comments_flag_test : RuntimeError
vm/dart/split_literals_test : RuntimeError
vm/dart/flutter_regress_93676_test : RuntimeError
vm/dart/use_trace_precompiler_flag_test : RuntimeError

@devoncarew
Copy link
Member

I'm not sure of the priority of this (blocking something?), but a git bisect may help to narrow down the cause.

@eseidel
Copy link
Contributor Author
eseidel commented Jun 19, 2024

True, git-bisect would. We've skipped the tests in our fork (or maybe someone will explain to us how we're supposed to run a non-named config?) so it's not high priority from our side.

@eseidel
Copy link
Contributor Author
eseidel commented Jun 19, 2024

Trying:

git bisect start 3.4.3 3.3.4
git bisect run test.sh

Where test.sh is:

# If we fail to sync or build, we exit with 125 to indicate that we can't
# test this commit.
gclient sync -D || exit 125
./tools/build.py --mode release create_sdk || exit 125
# Otherwise we run the tests and exit non-zero if they fail.
python3 tools/test.py -m release -c dartkp -r dart_precompiled -a arm64 --use-elf vm/dart/use_trace_precompiler_flag_test || exit 1

@eseidel
Copy link
Contributor Author
eseidel commented Jun 19, 2024
git bisect start
# status: waiting for both good and bad commits
# good: [d70d99a911b3316ca0d442caaf24fe57afe59893] Version 3.3.4
git bisect good d70d99a911b3316ca0d442caaf24fe57afe59893
# status: waiting for bad commit, 1 good commit known
# bad: [1a849f91804e73a830844a5bf546a3f7027fc36d] Version 3.4.3
git bisect bad 1a849f91804e73a830844a5bf546a3f7027fc36d
# good: [d70d99a911b3316ca0d442caaf24fe57afe59893] Version 3.3.4
git bisect good d70d99a911b3316ca0d442caaf24fe57afe59893
# bad: [1dba9e917c2e46523ad52cde5866fd2947f4dbcb] Deprecate some of the lookUp* methods
git bisect bad 1dba9e917c2e46523ad52cde5866fd2947f4dbcb
# good: [52eece210771ee04f7249aab9874a01ee26b47be] Skip test that hangs on reload bots.
git bisect good 52eece210771ee04f7249aab9874a01ee26b47be
# good: [e7f79cad7ccb3466c2c2bf0937a1ab49853afbaf] Macro. Report INVALID_MACRO_APPLICATION_TARGET.
git bisect good e7f79cad7ccb3466c2c2bf0937a1ab49853afbaf
# good: [628d744391f90a8254d585c38fbc1904a2c2065b] [dart:developer][VM/Service] Add APIs to dart:developer for recording HTTP profiling information, and for later retrieving that information
git bisect good 628d744391f90a8254d585c38fbc1904a2c2065b
# bad: [859e359266ef9fc71eab090fbc44532b581a2333] [macros] Cleanup after exception changes.
git bisect bad 859e359266ef9fc71eab090fbc44532b581a2333
# good: [c19c48d36267061942ab481eeb631ecc0ca62734] Revert "[deps] rev http, markdown, web_socket_channel"
git bisect good c19c48d36267061942ab481eeb631ecc0ca62734
# bad: [e82324990f19d150f822426963c086af5da12301] Bump web_socket_channel to 3db86bc0a09e1038a0fa418262c8a92211c5de69
git bisect bad e82324990f19d150f822426963c086af5da12301
# bad: [528e1092bc32fb3a54e46b9e268a49aa221c2119] [ddc] Cleanup dead code from the compiler
git bisect bad 528e1092bc32fb3a54e46b9e268a49aa221c2119
# bad: [0f83de7eda89c617bc294bf9e13287121b8ff027] linter: Migrate the curly_braces_in_flow_control_structures tests
git bisect bad 0f83de7eda89c617bc294bf9e13287121b8ff027
# bad: [ca9eb4af0f70fc7024f753e87b444ea22fe7f18e] [gardening/test] Cleanup AOT checks in tests
git bisect bad ca9eb4af0f70fc7024f753e87b444ea22fe7f18e
# good: [ee39f45f766d0ef21f4a7d4261f1d356ada015ff] linter: avoid a crash when running benchmarks w/o internet
git bisect good ee39f45f766d0ef21f4a7d4261f1d356ada015ff
# first bad commit: [ca9eb4af0f70fc7024f753e87b444ea22fe7f18e] [gardening/test] Cleanup AOT checks in tests

ca9eb4a is the culprit. @dcharkes

@dcharkes
Copy link
Contributor

@eseidel That CL should not influence how tests are run and cause FormatException: Must specify at least one of compiler or runtime in options or configuration name.

isVmAotConfiguration is merely a bool during the runtime of a test that makes tests exit early or not.

@eseidel
Copy link
Contributor Author
eseidel commented Jun 19, 2024

@eseidel That CL should not influence how tests are run and cause FormatException: Must specify at least one of compiler or runtime in options or configuration name.

isVmAotConfiguration is merely a bool during the runtime of a test that makes tests exit early or not.

I'm 100% certain it's that change. :) Unclear if I'm using test.py correctly, but the command line above works before that change and breaks after.

final Configuration _configuration = Configuration.parse(

FormatException: Must specify at least one of compiler or runtime in options or configuration name.
#0      Configuration.parse (package:smith/configuration.dart:251)
#1      _configuration (package:expect/config.dart:18)
#2      isVmAotConfiguration (package:expect/config.dart)
#3      isAOTRuntime (file:///Users/eseidel/Documents/GitHub/unmodified-dart-sdk/sdk/runtime/tests/vm/dart/use_flag_test_helper.dart:12)
#4      main (file:///Users/eseidel/Documents/GitHub/unmodified-dart-sdk/sdk/runtime/tests/vm/dart/use_trace_precompiler_flag_test.dart)
#5      _delayEntrypointInvocation.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:295)
#6      _RawReceivePort._handleMessage (dart:isolate-patch/isolate_patch.dart:184)

The configuration is attempted to parse, and it fails to figure out what the compiler is from "custom-configuration-1". Before ca9eb4a it didn't try to figure out the compiler/runtime from the configuration string, rather from Platform.executable.

@eseidel
Copy link
Contributor Author
eseidel commented Jun 19, 2024

python3 tools/test.py -m release -c dartkp -r dart_precompiled -a arm64 --use-elf vm/dart/use_trace_precompiler_flag_test works at ee39f45 and fails at ca9eb4a. I would very much welcome someone telling me that I'm using test.py wrong 🤷, but my guess is that we just need to change isVmAotConfiguration to use the old style check? Not sure.

@eseidel
Copy link
Contributor Author
eseidel commented Jun 19, 2024

It also may be a inside-google vs. outside thing. I don't know how the "named configurations" are determined, nor do I understand yet what about my test.py invocation which throws us off the named configuration path and into the "custom configuration" path where this hits 🤷

@dcharkes
Copy link
Contributor
dcharkes commented Jun 19, 2024

I never use -r, does $ tools/test.py -m release -c dartkp -a arm64 --use-elf vm/dart/use_trace_precompiler_flag_test work? Nope.

@dcharkes
Copy link
Contributor

The configuration is attempted to parse, and it fails to figure out what the compiler is from "custom-configuration-1". Before ca9eb4a it didn't try to figure out the compiler/runtime from the configuration string, rather from Platform.executable.

Ah right, my bad. I somehow interpreted this as the test runner failing, instead of the test failing. 😴 ☕

Only relying on the configuration string seems to be bad. I think package:smith should maybe also do a best effort if the config is custom or package:expect should wrap Configuration.parse in a try an fall back. The fallback for unknown configs should be path.basenameWithoutExtension(Platform.executable) == 'dart_precompiled_runtime'.

@dcharkes
Copy link
Contributor

@eseidel
Copy link
Contributor Author
eseidel commented Jun 19, 2024

This unfortunately doesn't seem to have fixed things.

If I try on main: (09e87e6)

 ./tools/build.py --mode debug create_sdk run_vm_tests
  python3 tools/test.py

It still immediately fails:

No build targets found.
Test configuration:
    custom-configuration-1(architecture: arm64, compiler: dartk, mode: debug, runtime: vm, system: mac)
Suites tested: corelib, ffi, kernel, language, lib, samples, standalone, utils, vm
[00:05 |   0% | +    7 | -    0]

FAILED: dartk-vm debug_arm64 vm/dart/build_id_test
Expected: Pass
Actual: RuntimeError

--- Command "vm" (took 03.000918s):
DART_CONFIGURATION=DebugARM64 xcodebuild/DebugARM64/dart --sound-null-safety -Dtest_runner.configuration=custom-configuration-1 --ignore-unrecognized-flags --packages=/Users/eseidel/Documents/GitHub/unmodified-dart-sdk/sdk/.dart_tool/package_config.json /Users/eseidel/Documents/GitHub/unmodified-dart-sdk/sdk/runtime/tests/vm/dart/build_id_test.dart

exit code:
255

stderr:
Unhandled exception:
FormatException: Must specify at least one of compiler or runtime in options or configuration name.
#0      Configuration.parse (package:smith/configuration.dart:264:9)
#1      _configuration (package:expect/config.dart:18:52)
#2      _configuration (package:expect/config.dart)
#3      isVmAotConfiguration (package:expect/config.dart:32:34)
#4      isAOTRuntime (file:///Users/eseidel/Documents/GitHub/unmodified-dart-sdk/sdk/runtime/tests/vm/dart/use_flag_test_helper.dart:12:22)
#5      isAOTRuntime (file:///Users/eseidel/Documents/GitHub/unmodified-dart-sdk/sdk/runtime/tests/vm/dart/use_flag_test_helper.dart)
#6      main (file:///Users/eseidel/Documents/GitHub/unmodified-dart-sdk/sdk/runtime/tests/vm/dart/build_id_test.dart:13:7)
#7      _delayEntrypointInvocation.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:297:19)
#8      _RawReceivePort._handleMessage (dart:isolate-patch/isolate_patch.dart:184:12)

--- Re-run this test:
python3 tools/test.py vm/dart/build_id_test
[00:05 |   0% | +    7 | -    1]

FAILED: dartk-vm debug_arm64 vm/dart/no_local_labels_test
Expected: Pass
Actual: RuntimeError

--- Command "vm" (took 05.000105s):

@eseidel
Copy link
Contributor Author
eseidel commented Jun 19, 2024

I don't have powers on this repo (including re-opening bugs, but I can file a new one if needed).

@dcharkes dcharkes reopened this Jun 19, 2024
@dcharkes
Copy link
Contributor

Ah, you're also trying to run as dartk, I tried make package:smith smart enough to recognize that dart.exe and dart are the VM. But the analyzer also runs dart.exe. Let me see if I can do more dirty tricks.

@dcharkes
Copy link
Contributor

@eseidel https://dart-review.googlesource.com/c/sdk/+/372360 should fix dartk.

Any other custom configurations you're using?

@eseidel
Copy link
Contributor Author
eseidel commented Jun 19, 2024

I'm not trying to swim upstream at all. :) If there is a better way I should be trying to run tests I'm happy to do that.

@dcharkes
Copy link
Contributor

It seems that (some) of our infra is rather configuration-name-dependent. Maybe we should be aiming on making a named configuration for every combination of flags? I'm not sure if that is even feasible. Thoughts @athomas @rmacnak-google?

@dart-lang dart-lang deleted a comment from AlnuimiAbdelkrim Jun 19, 2024
@eseidel
Copy link
Contributor Author
eseidel commented Jun 19, 2024

It does feel like the left and and right hand aren't talking to each other here. It's the test runner that is producing this configuration name, and then later another part of the test system which is assuming it's in a specific format. This presumably works inside Google because you're using "named" configurations? But I'm not sure how to set that up as someone who has made any modifications to the VM (also all my test cases above, are with unmodified VM checkouts from main, just changing what flags I do/don't pass to test.py). Happy to follow whatever rules, would just like them written down somewhere. :)

e.g. https://github.com/dart-lang/sdk/blob/main/docs/Testing-the-VM.md

@eseidel
Copy link
Contributor Author
eseidel commented Jun 19, 2024

@eseidel https://dart-review.googlesource.com/c/sdk/+/372360 should fix dartk.

Any other custom configurations you're using?

Thanks. I don't think we're even using that config, I just was trying to run "test.py" on main (without any flags) and hit that since I was still seeing failures on our bots after cherry picking your change from last night (thank you!) over to our fork this morning.

We use: ./tools/test.py -m debug --arch arm64 --progress line --compiler dartkp --use-elf --runtime dart_precompiled as pretty much our only test config (and -m release).

@rmacnak-google
Copy link
Contributor

Maybe we should be aiming on making a named configuration for every combination of flags? I'm not sure if that is even feasible.

Arbitrary VM flags can be part of combination of flags, and I don't think we want to start encoding all that into configuration names.

It seems most of these uses of the configuration name are tests that want to be skipped for JIT, so maybe moving AOT-only tests to a separate directory and using the status files to skip them mostly handles this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-test Cross-cutting test issues (use area- labels for specific failures; not used for package:test). triage-automation Issues triaged by automation (see https://github.com/dart-lang/sdk/wiki/Triage-automation) type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

5 participants