Skip to content

Support running tests by absolute file: uri #1893

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 20 commits into from
Feb 6, 2023
Merged

Support running tests by absolute file: uri #1893

merged 20 commits into from
Feb 6, 2023

Conversation

jakemac53
Copy link
Contributor

@jakemac53 jakemac53 commented Feb 1, 2023

Fixes #1891
Fixes #1899

For any path which looks like it has a scheme (contains "://"), parse it as a uri and grab the path from that. On windows, we also remove any leading / from the path because it will be followed by a drive letter, which we want to be the first part of the path (and not preceded by a /).

Open to better ideas here too, if you have them :)

@jakemac53 jakemac53 marked this pull request as draft February 1, 2023 20:45
@jakemac53 jakemac53 changed the title add a test for running by absolute file: uri Support running tests by absolute file: uri Feb 1, 2023
@jakemac53 jakemac53 marked this pull request as ready for review February 1, 2023 22:47
@jakemac53 jakemac53 requested a review from natebosch February 1, 2023 22:47
@jakemac53
Copy link
Contributor Author

ready for another review @natebosch

@jakemac53 jakemac53 merged commit ba6fb1c into master Feb 6, 2023
@jakemac53 jakemac53 deleted the file-uri-test branch February 6, 2023 23:22
@DanTup
Copy link
Contributor

DanTup commented Feb 7, 2023

On windows, we also remove any leading / from the path because it will be followed by a drive letter, which we want to be the first part of the path (and not preceded by a /).

You probably know this, but I think if you use .toFilePath() instead of .path you wouldn't need to do this (I've seen using .path on file URIs has been a source of many Windows bugs), although you'll get back the path in Windows form (with backslashes) which is often a good thing but I don't know if it's what you want here.

@DanTup
Copy link
Contributor

DanTup commented Feb 7, 2023

@jakemac53 it's not urgent, but so I know when to come back to the Flutter side of this, what's the process for it getting into the Dart SDK? If it's just a case of updating the revision in DEPs I'm happy to send a CL for that, but if there's a process/schedule (or you want to get other changes in with it) I'm happy to wait.

@jakemac53
Copy link
Contributor Author

It should just be updating the revision in DEPs, I think it might happen automatically now also but I am not sure. Usually we would wait until we publish as well but we probably could publish soon, cc @natebosch

@jakemac53
Copy link
Contributor Author

We also will want to validate these changes internally first

@DanTup
Copy link
Contributor

DanTup commented Feb 7, 2023

Okey, I'll hold off doing anything for a little bit. If you do roll it in (or notice it done automatically) and remember, let me know. Otherwise I'll check back in a little while :-)

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.

Running tests by line doesn't work on Windows "dart test file:///foo.dart" doesn't work without a query string
3 participants