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.

Sign up for GitHub

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

Jump to bottom

[nnbd_migration] unit tests fail with null safety enabled #43883

Closed
leafpetersen opened this issue Oct 21, 2020 · 5 comments
Closed

[nnbd_migration] unit tests fail with null safety enabled #43883

leafpetersen opened this issue Oct 21, 2020 · 5 comments
Assignees
@stereotype441
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P1 A high priority bug; for example, a single project is unusable or has many test failures

Comments

@leafpetersen
Copy link
Member

See failures here . It looks like these tests are generating snippets of dart code and running them through the migration tool, and when the experiment flag is turned on by default these snippets get treated as already opted in and the tests fail.

cc @srawlins @stereotype441 @devoncarew
@leafpetersen leafpetersen added the area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. label Oct 21, 2020
@leafpetersen leafpetersen mentioned this issue Oct 21, 2020
Closed
@franklinyow franklinyow added the P1 A high priority bug; for example, a single project is unusable or has many test failures label Oct 22, 2020
@srawlins srawlins self-assigned this Oct 22, 2020
@srawlins srawlins mentioned this issue Oct 22, 2020
Closed
dart-bot pushed a commit that referenced this issue Oct 23, 2020
@srawlins
Migrator: opt all test files _out_ of null safety.
360363e 
Well not quite all; migration_cli_test tests are a bit of a different
beast so I am going to tackle them separately. But this fixes over
1000 test failures, and should clean up test output for further
debugging of the flip CL.

Each test file needs to be in a package with a package config file
which spells out that it is _opted out_ of null safety.

Bug: #43883
Change-Id: I583f66119df57031fd80824111923e15e0f91782
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/168900
Reviewed-by: Leaf Petersen <leafp@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
dart-bot pushed a commit that referenced this issue Oct 23, 2020
@srawlins
Migrator: Prepare migration_cli_test for the flip CL.
9598c7b 
This is a weird CL because it strives to keep tests passing before and after
the CL, when they include a lot of error-path tests with weird pubspec files
and package config files.

Bug: #43883
Change-Id: I55dd4b398f3b1b53b8f2df95b2dcf880104ec7b6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/168901
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
@leafpetersen
Copy link
Member Author

These are green now, thanks!

@leafpetersen
Copy link
Member Author

I'm seeing this test failing still:

analyzer-unittest-asserts-release-win:pkg/nnbd_migration/test/front_end/region_renderer_test

@leafpetersen leafpetersen reopened this Oct 28, 2020
@leafpetersen
Copy link
Member Author

Logs here and here.

@stereotype441
Copy link
Member

I have a fix baking on trybots now: https://dart-review.googlesource.com/c/sdk/+/169421

@leafpetersen
Copy link
Member Author

Confirmed, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@stereotype441 stereotype441

Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P1 A high priority bug; for example, a single project is unusable or has many test failures
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@srawlins @stereotype441 @leafpetersen @franklinyow