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

#2350. Add/update factory constructor tests. Part 4 #2367

Merged
merged 5 commits into from
Dec 13, 2023
Merged

#2350. Add/update factory constructor tests. Part 4 #2367

merged 5 commits into from
Dec 13, 2023

Conversation

sgrekhov
Copy link
Contributor

No description provided.

eernstg
eernstg reviewed Nov 14, 2023
View reviewed changes
Copy link
Member

@eernstg eernstg left a comment

Choose a reason for hiding this comment

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

Looks good! There is one test, Language/Classes/Constructors/Factories/function_type_t06.dart, where the description refers to a compile-time error which is not the one that the test actually causes.

Apart from that there are couple of typos.

Finally, as the most complex issue, we need to decide on a useful approach to the discrepancy between the current language specification (https://github.com/dart-lang/language/blob/main/specification/dartLangSpec.tex) which is describing a language where null safety has only been introduced in a few chapters, and a future language specification where null safety has been fully integrated (which is probably going to be quite similar to dart-lang/language#2605).

Language/Classes/Constructors/Factories/function_type_t06.dart Outdated Show resolved Hide resolved
Language/Classes/Constructors/Factories/redirecting_constructor_t01.dart Show resolved Hide resolved
Language/Classes/Constructors/Factories/return_type_A03_t01.dart Outdated Show resolved Hide resolved
Language/Classes/Constructors/Factories/return_type_A03_t02.dart Outdated Show resolved Hide resolved
Language/Classes/Constructors/Factories/return_type_A03_t03.dart Outdated Show resolved Hide resolved
sgrekhov
sgrekhov commented Nov 14, 2023
View reviewed changes
Copy link
Contributor Author

@sgrekhov sgrekhov left a comment

Choose a reason for hiding this comment

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

Updated. Please take another look

Language/Classes/Constructors/Factories/function_type_t06.dart Outdated Show resolved Hide resolved
Language/Classes/Constructors/Factories/redirecting_constructor_t01.dart Show resolved Hide resolved
Language/Classes/Constructors/Factories/return_type_A03_t01.dart Outdated Show resolved Hide resolved
eernstg
eernstg reviewed Dec 11, 2023
View reviewed changes
Copy link
Member

@eernstg eernstg left a comment

Choose a reason for hiding this comment

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

Looks good! Just a couple of typos and a suggested addition to a comment.

Language/Classes/Constructors/Factories/accessible_class_A01_t03.dart Outdated Show resolved Hide resolved
Language/Classes/Constructors/Factories/parameter_type_t01.dart Outdated Show resolved Hide resolved
Language/Classes/Constructors/Factories/redirecting_constructor_t01.dart Show resolved Hide resolved
Language/Classes/Constructors/Factories/return_type_A03_t01.dart Outdated Show resolved Hide resolved
@sgrekhov
Copy link
Contributor Author

Updated. Please review

eernstg
eernstg reviewed Dec 12, 2023
View reviewed changes
Copy link
Member

@eernstg eernstg left a comment

Choose a reason for hiding this comment

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

Thanks! The new test needs to make a distinction between the formal parameter types of the redirecting factory constructor and the redirectee, I've commented on that.

(Alternatively, I just haven't understood what 'parameter_type_t01.dart' is doing ;-).

@sgrekhov
Copy link
Contributor Author

parameter_type_t01 updated, parameter_type_t02 deleted because the first test does all the work now. Please review

eernstg
eernstg approved these changes Dec 13, 2023
View reviewed changes
Copy link
Member

@eernstg eernstg left a comment

Choose a reason for hiding this comment

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

LGTM!

@eernstg eernstg merged commit d5d1df8 into dart-lang:master Dec 13, 2023
1 of 2 checks passed
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Dec 19, 2023
@sgrekhov
[co19] Roll co19 to b80c9a4cb6fb22b5c8a7395d532c8e461fe33ab9
8f6be08 
2023-12-19 sgrekhov22@gmail.com Fixes dart-lang/co19#2441. Fix roll failures (dart-lang/co19#2443)
2023-12-18 sgrekhov22@gmail.com Fixes dart-lang/co19#2435. Fix roll failures (dart-lang/co19#2437)
2023-12-18 sgrekhov22@gmail.com dart-lang/co19#2420. Add extension types exhaustiveness tests. Variables (dart-lang/co19#2431)
2023-12-15 sgrekhov22@gmail.com Fixes dart-lang/co19#2430. Update expected errors positions for CFE (dart-lang/co19#2433)
2023-12-15 sgrekhov22@gmail.com Fixes dart-lang/co19#2432. Remove excessive expected error for CFE (dart-lang/co19#2434)
2023-12-14 sgrekhov22@gmail.com dart-lang/co19#2420. Add extension types exhaustiveness tests. Maps (dart-lang/co19#2426)
2023-12-14 sgrekhov22@gmail.com dart-lang/co19#2420. Add extension types exhaustiveness tests. Lists (dart-lang/co19#2424)
2023-12-14 sgrekhov22@gmail.com dart-lang/co19#2139. Fix wrong failure of Language/Functions/element_type_A02_t06 (dart-lang/co19#2429)
2023-12-13 sgrekhov22@gmail.com dart-lang/co19#2350. Add more factory constructors tests (dart-lang/co19#2427)
2023-12-13 sgrekhov22@gmail.com Fixes dart-lang/co19#2415. Update `StreamController.broadcast()` test according to the changed documentation (dart-lang/co19#2425)
2023-12-13 sgrekhov22@gmail.com dart-lang/co19#2350. Add/update factory constructor tests. Part 4 (dart-lang/co19#2367)
2023-12-12 sgrekhov22@gmail.com dart-lang/co19#2420. Add extension types exhaustiveness tests. Enums, trivial cases (dart-lang/co19#2423)
2023-12-11 sgrekhov22@gmail.com dart-lang/co19#2420. Add extension types exhaustiveness tests. Enums (dart-lang/co19#2422)
2023-12-11 sgrekhov22@gmail.com dart-lang/co19#2386. Rename well-bounded tests. Update descriptions (dart-lang/co19#2405)
2023-12-08 sgrekhov22@gmail.com dart-lang/co19#2415. Change expectations for Stream.asyncMap() according to the current behavior (dart-lang/co19#2421)

Change-Id: I777eba4f1615c8477a5d2044f295696dfc210b1d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/342582
Commit-Queue: Erik Ernst <eernst@google.com>
Reviewed-by: Alexander Thomas <athom@google.com>
Reviewed-by: Erik Ernst <eernst@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eernstg eernstg eernstg approved these changes

@chloestefantsova chloestefantsova Awaiting requested review from chloestefantsova

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@sgrekhov @eernstg