-
Notifications
You must be signed in to change notification settings - Fork 214
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
Add test coverage for test suite import prefix value #2226
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While this could work, this is definitely not intended to be a public part of our API, it is an implementation detail only.
So from that perspective I would definitely prefer any other option. If there is no other solution that works, we could consider it.
This may not work for other integrations either - such as build_test, flutter, google3, or any other custom integrations with package:test.
cc @natebosch |
Note that this also currently precludes us from adding a |
How so? |
I believe the code I saw was looking for |
I left a comment on the other issue, but we don't currently restrict test suites to those under the package root, so I don't think you can guarantee that this URI will be useful. |
We are checking the prefix directly, so this should handle whitespace changes like adding |
Will hold off on landing this until the discussion at flutter/flutter#143170 (comment) is resolved. |
Closing this since this PR since this issue was resolved with a different solution than using the test import prefix: #2246 |
Addresses a TODO from flutter/flutter#143170 (comment). The matching Flutter Tools PR is here: flutter/flutter#148244