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

#2386. Rename well-bounded tests. Update descriptions #2405

Merged
merged 4 commits into from
Dec 11, 2023

Conversation

sgrekhov
Copy link
Contributor

I stashed my changes and some information about renaming was lost. Here it is

class_A01_t01 => well_bounded_A01_t01
class_A01_t02 => well_bounded_A01_t02
class_A01_t03 => well_bounded_A01_t03
function_A01_t01 => well_bounded_A02_t01
function_A01_t02 => well_bounded_A02_t02
function_A01_t03 => well_bounded_A02_t03
typedef1_A01_t01 => well_bounded_A03_t01
typedef1_A01_t04 => well_bounded_A03_t04

Before adding the new tests I checked the existing ones and found that these tests don't follow the accepted naming pattern that file_name_AXX should contain the same assertion text (A in a tests name means Assertion). So, usually test_name_A01_t* have one assertion text, test_name_A02_t* have the next assertion from the same chapter/paragraph etc...

Here an exception from this rule is made for the convinience. All well_bounded_A* tests have the same assertion text, but _A01_t* check classes, _A02_t* functions etc. It's much easer to maintain the renamed tests

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.

I think we should revisit the wording of a bunch of tests. They are getting close, but they tend to say "well-bounded" about everything, and we are actually looking at 3-4 different kinds of rules and corresponding errors.

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.

Thank you! Descriptions are updated. With this PR I stepped onto a land of superbounded types which is not well-familiar to me

@eernstg
Copy link
Member

eernstg commented Nov 28, 2023

I don't think anybody has super-bounded types completely under the skin. 😸

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.

Guess what, I found a bunch of other things to complain about. But it's just about little things now. ;-)

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.

Descriptions and comments updated. Thank you! Please take another look

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.

Just one typo left!

@sgrekhov
Copy link
Contributor Author

Typo fixed, thank you!

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, thanks!

@eernstg eernstg merged commit 21a5fc1 into dart-lang:master Dec 11, 2023
2 checks passed
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Dec 19, 2023
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants