-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Move analyzer test utilities to analyzer_utilities package #55660
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
Comments
It's quite possible that some of the base classes might include support that isn't needed by the tests that a plugin author would need to write. It might be worthwhile to start by thinking about what the requirements are and what those pieces of support actually depend on. Moving the rest of the support out to mixins might reduce the number of dependencies. It's also the case that plugin authors will need to be able to do some things that we currently don't have support for. For example, here are some initial thoughts. Plugin authors need to be able to test both lints and fixes. For both of those use cases the tests need to be able to create the code being tested and any supporting code in an in-memory disk layout. That includes being able to create analysis options files, package config files, and any (mock) packages that the test code needs to be able to depend on (including packages only available to the plugin author). For lint tests they need to be able to specify which file(s) to analyze and the diagnostics that the lint is expected to produce. They'll also need to be able to register the lint rule being tested. For fix tests they need to do every they need for testing lint rules as well as confirming the result of applying the fix to the appropriate files. They probably don't need the printers that we use for printing resolution output, the Also, are you thinking of |
Either. I hadn't thought of the differences. Something that may make this task difficult is that much of ContextResolutionTest (and others I mentioned) depends on package:analyzer. It uses Anyways, I think that means that |
Another anecdote pointing to the need to solve this: I recently moved So I think we need an
I wrote, "Either. I hadn't thought of the differences." Now I'm thinking a fresh, test-oriented package (like, the name is also test-oriented) is warranted. We wouldn't need to publish this package for a while. Maybe not until plugin authors need it. |
Work towards #55660 Change-Id: Ia4f8186c328c9929869d1305ee0fe606d9dfa0e5 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/425320 Commit-Queue: Samuel Rawlins <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]> Reviewed-by: Alexander Thomas <[email protected]>
We intend to publish and maintain this as a set of testing-related utilities for the analyzer packages and for analyzer plugins. Work towards #55660 See the doc: https://docs.google.com/document/d/1jRtd8B1ijPAP6Pz89HRnyIZXw2VMjaZx0vRZTpoNO84/edit?tab=t.0#heading=h.2sz41a544qhi Change-Id: I2764b1357a932fa955060b26d78038997eaa9536 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/425080 Reviewed-by: Slava Egorov <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]> Reviewed-by: Paul Berry <[email protected]>
Only one test implementation used it, and it can be a proper function instead of a getter that returns a function. Work towards #55660 Change-Id: I9f6e6d4c6f88c0b613c69abec6b714ef5fe4bf64 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/426401 Auto-Submit: Samuel Rawlins <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Brian Wilkerson <[email protected]>
Work towards #55660 Change-Id: Ic789b8347d3bfc71a02af423fbb1a8c58da7b32d Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/424921 Reviewed-by: Ivan Inozemtsev <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]>
Quick background / example: I would like to move
ResolveNameInScopeTest
from the analyzer package to the linter package. But it depends onAbstractLinterContextTest
(also inpkg/analyzer/test
) which depends on PubPackageResolutionTest (also inpkg/analyzer/test
). So I cannot move any tests until we have some shared base classes. Or I can copy swaths of code.Creating shared testing code between the analyzer package, linter package, analysis_server and analysis_server_plugin packages is necessary for the plugins story. CC @bwilkerson @scheglov @pq
ContextResolutionTest
It would take significant investigation to determine how many test utility classes and mixins would be prudent to move. But as a starting place, I think it would be of huge benefit to move
ContextResolutionTest
intopackage:analyzer_utilities
. How do we get there?ResourceProviderMixin
ResourceProviderMixin
(mixed intoContextResolutionTest
) is declared inpackage:analyzer/src/test_utilities
, so we're already in a good place there. Should be easier to move than other things, as it cannot depend on code inpkg/analyzer/test
. This might be the best starting place.ResolutionTest
ContextResolutionTest
also mixes inResolutionTest
, so this is also a pre-requirement. Unfortunately it lives inpkg/analyzer/test
, and is in a file which imports eight other files frompkg/analyzer/test
. Yikes.PubPackageResolutionTest
PubPackageResolutionTest
is also an important base class, as it is the base for supporting analysis options files and package configs.The text was updated successfully, but these errors were encountered: