#2559. Add tests for augment libraries visibility #2561
Conversation
sgrekhov
requested review from
eernstg,
athomas and
chloestefantsova
and removed request for
athomas
There was a problem hiding this comment.
Looks good!
I would hope that we could use the augmented ("owner") library as the entry point in every case, because I'm arguing that it should just be an error to execute an augmentation library (dart-lang/language#3642).
(That's a messy feature, anyway.)
Doesn't the test runner support detecting errors that occur in some other file than the library which is specified as "the library to execute" (i.e., the entry point)? Just checking, https://dart-review.googlesource.com/c/sdk/+/345541 has been merged. I would think that this makes it possible to use the augmented library as "the file to execute" every time.
LanguageFeatures/Augmentation-libraries/defining_augmentation_A01_t02.dart
Outdated
Show resolved
Hide resolved
LanguageFeatures/Augmentation-libraries/defining_augmentation_A02_t02.dart
Show resolved
Hide resolved
Test runner doesn't support expecting errors in augmentation now. Therefore, some tests from this PR fails in spite of that augmentation has expected error it anyway reported as unexpected. Do we have any plans for supporting expected errors in augmentations? |
|
Ok, now test runner does support expected errors in augmented libraries. This change is ready for review now |
There was a problem hiding this comment.
Looks good, with a couple of typos.
One thing came up: I think it would be useful to use an imported namespace in one case, because there could be a kind of bug where unused import namespaces are sort of ignored, and then we could get a successful test run for the wrong reason. ;-)
LanguageFeatures/Augmentation-libraries/defining_augmentation_A02_t02.dart
Show resolved
Hide resolved
|
|
||
| library augment 'defining_augmentation_A02_t04.dart'; | ||
|
|
||
| import 'augmentation_libraries_lib.dart'; |
There was a problem hiding this comment.
Perhaps this was intended?:
| import 'augmentation_libraries_lib.dart'; | |
| import 'augmentation_libraries_lib.dart'; | |
| export 'augmentation_libraries_lib.dart'; |
There was a problem hiding this comment.
Perhaps use AL in this library, just to make sure that the import really occurs (importing into an augmentation library may be new code in the tools, so maybe they could do nothing for an import which isn't used).
There was a problem hiding this comment.
I looked at LanguageFeatures/Augmentation-libraries/defining_augmentation_A02_t04.dart and concluded from the @description that there should be an export somewhere, and this was the only location where it would fit, as far as I could see.
On the other hand, the @description in this file does not mention any exports. Maybe the description should be adjusted in one or the other file?
LanguageFeatures/Augmentation-libraries/defining_augmentation_A02_t04_lib2.dart
Outdated
Show resolved
Hide resolved
LanguageFeatures/Augmentation-libraries/defining_augmentation_A02_t04.dart
Outdated
Show resolved
Hide resolved
|
Thank you! Updated. Please take another look |
|
PR updated, |
|
|
||
| library augment 'defining_augmentation_A02_t04.dart'; | ||
|
|
||
| import 'augmentation_libraries_lib.dart'; |
There was a problem hiding this comment.
I looked at LanguageFeatures/Augmentation-libraries/defining_augmentation_A02_t04.dart and concluded from the @description that there should be an export somewhere, and this was the only location where it would fit, as far as I could see.
On the other hand, the @description in this file does not mention any exports. Maybe the description should be adjusted in one or the other file?
|
Thank you. Updated |
2024-04-15 [email protected] Fixes dart-lang/co19#2609. Fix runtime failure and typos (dart-lang/co19#2610) 2024-04-15 [email protected] Fixes dart-lang/co19#2606. Fix the new roll failures (dart-lang/co19#2608) 2024-04-15 [email protected] dart-lang/co19#2559. Add tests for `augmented()` expression (dart-lang/co19#2601) 2024-04-12 [email protected] Fixes dart-lang/co19#2604. Fix new roll failures, add issues numbers (dart-lang/co19#2605) 2024-04-12 [email protected] Fixes dart-lang/co19#2602. Fix new roll failures (dart-lang/co19#2603) 2024-04-11 [email protected] dart-lang/co19#2559. Add extensions to augmenting types tests (dart-lang/co19#2600) 2024-04-10 [email protected] dart-lang/co19#2559. Add augmenting functions tests (dart-lang/co19#2599) 2024-04-10 [email protected] dart-lang/co19#2559. Add tests for operators (dart-lang/co19#2598) 2024-04-09 [email protected] dart-lang/co19#2559. Add more augmenting types tests (dart-lang/co19#2596) 2024-04-09 [email protected] dart-lang/co19#2559. Make enum augmenting types tests stronger (dart-lang/co19#2597) 2024-04-09 [email protected] dart-lang/co19#2559. Add tests for augment libraries visibility (dart-lang/co19#2561) 2024-04-09 [email protected] dart-lang/co19#2559. Add augmenting types tests. Part 3 (dart-lang/co19#2570) 2024-04-08 [email protected] dart-lang/co19#2559. Update/add new defining augmentation tests (dart-lang/co19#2595) 2024-04-08 [email protected] dart-lang/co19#2559. Add more augmenting types tests for `with` clause (dart-lang/co19#2594) 2024-04-05 [email protected] Update Symbol tests documentation (dart-lang/co19#2593) 2024-04-05 [email protected] dart-lang/co19#2559. Add more augmenting types tests for `on` clause (dart-lang/co19#2592) 2024-04-05 [email protected] dart-lang/co19#2559. Add more augmenting types tests for `extends` and `implements` clauses (dart-lang/co19#2591) Change-Id: I7e5c91f71535f69fe628460d89c227a54080533e Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/362803 Reviewed-by: Alexander Thomas <[email protected]> Reviewed-by: Jonas Termansen <[email protected]>
No description provided.