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

Run non-null-safe tests with Dart 2 #791

Merged
merged 4 commits into from
Jan 18, 2023
Merged

Run non-null-safe tests with Dart 2 #791

merged 4 commits into from
Jan 18, 2023

Conversation

kevmoo
Copy link
Collaborator

@kevmoo kevmoo commented Jan 17, 2023

Move the non-null-safe usage of the API to another file, test the file with Dart 2.x.

@kevmoo kevmoo requested a review from mraleph January 17, 2023 19:19
@kevmoo
Copy link
Collaborator Author

kevmoo commented Jan 17, 2023

Feel free to squash/merge and delete the branch if you're happy

@mraleph mraleph requested a review from osa1 January 17, 2023 19:24
@mraleph
Copy link
Member

mraleph commented Jan 17, 2023

@osa1 I think this might be related to what you looked at today.

@osa1
Copy link
Member

osa1 commented Jan 17, 2023

@kevmoo we've been discussing these tests in Dart Ecosystem chat. I don't think we want to remove these tests yet as internally we have non-null-safe Dart that uses this library.

@kevmoo
Copy link
Collaborator Author

kevmoo commented Jan 17, 2023

@osa1 – but we can't run Dart 3.0.0- tests against this package w/ them there.

We could pull those out into another test file and only run them on stable or something 🤷

@osa1
Copy link
Member

osa1 commented Jan 17, 2023

Right, I think we will need to move these to another file/package, and configure mono_repo to only test that package/file with SDK 2.x.

Apparently dart:test has a separate packages for legacy tests, for example https://github.com/dart-lang/test/blob/master/legacy_tests/spawn_hybrid_with_optout/pubspec.yaml. We could do the same. If we can specify running a specific file with a specific SDK in mono_repo.yaml we could also just move the file outside of test/ (maybe to legacy_test/), that would be simpler.

@kevmoo
Copy link
Collaborator Author

kevmoo commented Jan 17, 2023

@osa1 – sure? Just trying to get CI to green so we can keep the signals of knowing when the SDK breaks things.

@osa1 osa1 changed the title Drop legacy null tests Run non-null-safe tests with Dart 2 Jan 18, 2023
@osa1
Copy link
Member

osa1 commented Jan 18, 2023

It seems like on CI dart analyze analyzes the new file. I don't understand why that happens, locally when I run dart analyze with Dart 3.0.x it doesn't analyze the new file.

We need to somehow make dart analyze skip the file with dev SDK.

Copy link
Member

@osa1 osa1 left a comment

Choose a reason for hiding this comment

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

LGTM now, but someone should review my mono_repo changes :-)

Copy link
Collaborator

@devoncarew devoncarew left a comment

Choose a reason for hiding this comment

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

mono_repo changes and repo reconfiguration lgtm

@kevmoo kevmoo merged commit cc0f287 into master Jan 18, 2023
@kevmoo kevmoo deleted the no_legacy branch January 18, 2023 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants