Skip to content

#2641. Add patterns tests #2682

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

Merged
merged 3 commits into from
May 23, 2024
Merged

#2641. Add patterns tests #2682

merged 3 commits into from
May 23, 2024

Conversation

sgrekhov
Copy link
Contributor

No description provided.

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! However, it isn't entirely obvious to me why these tests have a relationship with the wildcarded variable feature, and why they aren't just duplicates of existing tests about patterns?

(There's a lot of similar elements in 'LanguageFeatures/Patterns/wildcards_A*.dart').

I think there was an interaction between wildcard patterns and wildcarded variable declarations that came up recently, but I can't remember it right now. Is there something which is actually new here?

@sgrekhov
Copy link
Contributor Author

Yes, there are duplicated elements but not entierly. These tests check wildcards in more details, in particular wildcards assignment. Patterns tests more tests switch statement and no fully tests the assignment. There are also no tetst that check that initializer is executed anyway in case of wildcarded assignment etc. I think it's Ok to have these, partially intersecting tests.

But if you insists I can move them to the Patterns folder

@sgrekhov sgrekhov requested a review from eernstg May 23, 2024 10:39
@eernstg
Copy link
Member

eernstg commented May 23, 2024

OK, thanks!

I think it would indeed be helpful to move them to Patterns, considering that there are no wildcarded variables in these tests.

@sgrekhov
Copy link
Contributor Author

No problem. Please review

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 21fed8d into dart-lang:master May 23, 2024
2 checks passed
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request May 24, 2024
2024-05-24 [email protected] Fixes dart-lang/co19#2684. Update expected errors positions for CFE (dart-lang/co19#2685)
2024-05-24 [email protected] dart-lang/co19#2641. Add wildcards do not shadow tests (dart-lang/co19#2683)
2024-05-23 [email protected] dart-lang/co19#2641. Add patterns tests (dart-lang/co19#2682)
2024-05-22 [email protected] dart-lang/co19#2641. Check that it is still an error to declare more than one enum member named `_` (dart-lang/co19#2674)
2024-05-22 [email protected] dart-lang/co19#2641. Add import prefix tests (dart-lang/co19#2681)
2024-05-22 [email protected] dart-lang/co19#2641. Add record types tests (dart-lang/co19#2679)
2024-05-22 [email protected] dart-lang/co19#2641. Add local functions test (dart-lang/co19#2680)
2024-05-22 [email protected] dart-lang/co19#2641. Check that it is still an error to declare more than one extension type member named `_` (dart-lang/co19#2675)
2024-05-21 [email protected] dart-lang/co19#2641. Add late variables named `_` tests (dart-lang/co19#2677)
2024-05-21 [email protected] dart-lang/co19#2641. Check that it is still an error to declare more than one extension member named _ (dart-lang/co19#2676)
2024-05-19 [email protected] Fixes dart-lang/co19#2660. Add missing import to augmented_expression_A01_t05_lib.dart (dart-lang/co19#2673)
2024-05-17 [email protected] dart-lang/co19#2641. Check that it is still an error to declare more than one mixin member named `_` (dart-lang/co19#2672)
2024-05-17 [email protected] dart-lang/co19#2641. Wildcards. Update expected errors positions for CFE (dart-lang/co19#2671)

Change-Id: Ia94383cb2dc2a5912950ee3e13ee93a9576f3500
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/367963
Reviewed-by: Alexander Thomas <[email protected]>
Reviewed-by: Erik Ernst <[email protected]>
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.

2 participants