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

dummy_compiler_test and recursive_import_test are extremely slow on dart2js-linux-d8-hostchecked #30773

Open
athomas opened this issue Sep 18, 2017 · 4 comments
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. web-dart2js

Comments

@athomas
Copy link
Member

athomas commented Sep 18, 2017

Skipping the dummy_compiler_test and recursive_import_test reduces the runtime for the dart2js extra tests by 5 minutes (10 minutes total because they are run in checked and unchecked mode) on the dart2js-linux-d8-hostchecked-try builder.

@sigmundch @efortuna I'm adding Skip for that configuration to the status file for now, is there anything that could be done to make these tests execute faster? Is it important to have them in the CQ?

https://dart-review.googlesource.com/c/sdk/+/6400

@athomas athomas added web-dart2js area-infrastructure Use area-infrastructure for SDK infrastructure issues, like continuous integration bot changes. labels Sep 18, 2017
@sigmundch
Copy link
Member

Thanks @athomas To be honest I think it's fine to suppress it in host-checked. These tests were intended as tests of the compiler API, and I'd be fine just testing this in the VM and not running this on any other configuration (so in the dart2js + d8 config seems rather unnecessary). @johnniwinther - do you agree?

@johnniwinther
Copy link
Member

recursive_import_test is not important for CQ nor dart2js+d8. It tests part of the old frontend library loader.

dummy_compiler_test doesn't need to be on CQ but should be run on dart2js+d8. It is not interesting on the vm: It is a self-compilation test and we already know that the vm can run dart2js.

@athomas athomas self-assigned this Sep 19, 2017
@athomas
Copy link
Member Author

athomas commented Sep 19, 2017

Ok, I will re-enable them for buildbot as soon as we have a way to skip them only for the CQ.

@sigmundch
Copy link
Member

OK - I sent https://dart-review.googlesource.com/c/sdk/+/45364 to:
(a) delete recursive_import_test
(b) document better the intent of dummy_compiler_test

I think we should eventually move dummy_compiler_test out of dart2js_extra into our actual unit test suite, and similar to our stacktrace tests, we should invoke d8 directly with it instead.

dart-bot pushed a commit that referenced this issue Mar 7, 2018
recursive_import: was covering logic for the old frontend and it is no longer
relevant.  dummy_compiler is not just about APIs, is about self-hosting.

TBR=sra@google.com

Bug: #30773
Change-Id: I56d094d9ecee47264c7a7a206f942d9e3dbdaadc
Reviewed-on: https://dart-review.googlesource.com/45364
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Sigmund Cherem <sigmund@google.com>
@athomas athomas removed their assignment Apr 2, 2019
@athomas athomas removed the area-infrastructure Use area-infrastructure for SDK infrastructure issues, like continuous integration bot changes. label Apr 2, 2019
@vsmenon vsmenon added the area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. label Jul 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. web-dart2js
Projects
None yet
Development

No branches or pull requests

4 participants