Skip to content

Add language version comments to bootstrap files - #1240

Merged
jakemac53 merged 7 commits into
null_safetyfrom
bootstrap-language-version
May 20, 2020
Merged

jakemac53 merged 7 commits into
null_safetyfrom
bootstrap-language-version

Conversation

@jakemac53

Copy link
Copy Markdown
Contributor
  • Adds a nullable languageVersionComment field to Metadata
  • Adds a utility to get a language comment that matches the root packages version
  • Adds explicit language version comments to all bootstrap files ensuring the versions matches that of the test (note this assumes tests live in the root package)
  • Clean up the handling of serving bootstrap dart files in the browser platform which had diverged and was serving the wrong content.

Closes #1236

I am not sure of the proper way to test this exactly, suggestions welcome. I manually tested the vm stuff and it seems to work correctly in terms of opting in/out of strong mode.

@jakemac53
jakemac53 requested a review from natebosch May 19, 2020 21:52
@jakemac53

Copy link
Copy Markdown
Contributor Author

Ok I pushed some tests here, I opted for integration tests that rely on the CFE error about having non-opted in libraries in strong mode which seems pretty reliable long term.

@jakemac53

jakemac53 commented May 20, 2020 •

Copy link
Copy Markdown
Contributor Author

Note that a lot of other tests are failing now, I am going to start looking into that. I think its mostly integration tests getting opted in due to not having a language version default.

This is just failing through pub run test though so I don't think it is blocking for this PR.

@jakemac53 jakemac53 assigned jakemac53 and natebosch and unassigned jakemac53 May 20, 2020
Comment thread pkgs/test/test/runner/runner_test.dart Outdated
Comment thread pkgs/test/test/runner/runner_test.dart Outdated
Comment thread pkgs/test/test/runner/runner_test.dart Outdated
test.stdout,
containsInOrder([
'Unable to spawn isolate:',
'Error: A library can\'t opt out of non-nullable by default, when in nnbd-strong mode.'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this our message, or is it coming from somewhere else?

cc @kevmoo - how important is it that our user facing messages use "sound" instead of "strong"?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is likely originating from the CFE.

@jakemac53
jakemac53 merged commit 5312b23 into null_safety May 20, 2020
@jakemac53
jakemac53 deleted the bootstrap-language-version branch May 20, 2020 18:57
jakemac53 added a commit that referenced this pull request May 20, 2020
This starts getting web tests compiling through the test runner, if used in combination with #1240.

However I see some late initialization errors that need to be tracked down still, can follow up with that later.
jakemac53 added a commit that referenced this pull request Jun 16, 2020
- Adds a nullable `languageVersionComment` field to `Metadata`
- Adds a utility to get a language comment that matches the root packages version
- Adds explicit language version comments to all bootstrap files ensuring the versions matches that of the test (note this assumes tests live in the root package)
- Clean up the handling of serving bootstrap dart files in the browser platform which had diverged and was serving the wrong content.
jakemac53 added a commit that referenced this pull request Jun 16, 2020
- Adds a nullable `languageVersionComment` field to `Metadata`
- Adds a utility to get a language comment that matches the root packages version
- Adds explicit language version comments to all bootstrap files ensuring the versions matches that of the test (note this assumes tests live in the root package)
- Clean up the handling of serving bootstrap dart files in the browser platform which had diverged and was serving the wrong content.
jakemac53 added a commit that referenced this pull request Jun 16, 2020
* Add language version comments to bootstrap files (#1240)

- Adds a nullable `languageVersionComment` field to `Metadata`
- Adds a utility to get a language comment that matches the root packages version
- Adds explicit language version comments to all bootstrap files ensuring the versions matches that of the test (note this assumes tests live in the root package)
- Clean up the handling of serving bootstrap dart files in the browser platform which had diverged and was serving the wrong content.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants