Skip to content

Conversation

natebosch
Copy link
Member

Closes #2535

Add a check for the workspace_ref.json file in place of the
pubspec.lock file that exists for single package pub solutions.

When the workspace ref exists, follow it and read the
package_graph.json file from the workspace root.

Closes #2535

Add a check for the `workspace_ref.json` file in place of the
`pubspec.lock` file that exists for single package pub solutions.

When the workspace ref exists, follow it and read the
`package_graph.json` file from the workspace root.
@natebosch
Copy link
Member Author

@jonasfj - do you have any concerns about the test running reading workspace_ref.json and package_graph.json files?

Copy link

github-actions bot commented Sep 4, 2025

PR Health

Changelog Entry ✔️
Package Changed Files

Changes to files need to be accounted for in their respective changelogs.

This check can be disabled by tagging the PR with skip-changelog-check.

@DanTup
Copy link
Contributor

DanTup commented Sep 5, 2025

@natebosch I just noticed this in the comments:

/// This is a semantic version, optionally followed by a space and additional
/// data about its source.

The code just seems to return the version from the JSON. Do you know in what circumstances this has a space and additional data? (I think Dart-Code is just assuming it's always a valid version right now, and if this comment is still accurate I might need to update it).

@natebosch
Copy link
Member Author

Do you know in what circumstances this has a space and additional data?

The existing code path that reads from pubspec.lock can return extra data.

case 'git':
var version = package['version'];
if (version is! String) return null;
var description = package['description'];
if (description is! Map) return null;
var ref = description['resolved-ref'];
if (ref is! String) return null;
return '$version (${ref.substring(0, 7)})';
case 'path':
var version = package['version'];
if (version is! String) return null;
var description = package['description'];
if (description is! Map) return null;
var path = description['path'];
if (path is! String) return null;
return '$version (from $path)';

I didn't replicate it mostly because I don't expect it to be worth the effort. Barely anyone uses a git or path dependency on the test runner. I think it's safe enough for dart-code to assume it will only ever see a semver version there.

If removing it doesn't break much on CI we could also drop that behavior entirely.

The test had been passing because the `testVersion` variable was null in
the pub workspace which matched the behavior of the test_descriptor
package within the test..
natebosch added a commit that referenced this pull request Sep 6, 2025
Follow up to #2538

This is only relevant when using the test runner as a git or path
dependency. In these cases the version will already end in `-WIP` with
out current development practices and in the places where it may come up
the users don't need help understanding where the code is coming from.
@natebosch natebosch marked this pull request as ready for review September 6, 2025 00:18
@natebosch natebosch requested a review from a team as a code owner September 6, 2025 00:18
@natebosch
Copy link
Member Author

If removing it doesn't break much on CI we could also drop that behavior entirely.

Checking the feasibility in #2539

@natebosch
Copy link
Member Author

@sigurdm do you have any concerns about this usage? Are workspace_ref.json and package_graph.json stable enough to rely on this way?

@DanTup
Copy link
Contributor

DanTup commented Sep 8, 2025

Barely anyone uses a git or path dependency on the test runner. I think it's safe enough for dart-code to assume it will only ever see a semver version there.

Ah, thanks for the info. I'll leave Dart-Code as it is then. We do handle when we can't parse a version from the output and it sounds like you might remove this anyway. If it can't be removed and it turns out to be a problem, I'll just trim after the first space.

@sigurdm
Copy link
Contributor

sigurdm commented Sep 8, 2025

@jonasfj - do you have any concerns about the test running reading workspace_ref.json and package_graph.json files?

I don't think reading workspace_ref is any big issue. Flutter tools already relies on the file being there, so it is probably not going anywhere. But I think it would be easier to use https://api.dart.dev/dart-isolate/Isolate/packageConfigSync.html to find the package config, and then find package_graph.json next to that.

You can rely on the package_graph always being present, so if you are ok with just the version number (no information about the source), you should be able to leave out the code reading pubspec.lock

@sigurdm
Copy link
Contributor

sigurdm commented Sep 8, 2025

And, alternatively, if you want to always read the pubspec.lock file, it will always be present in the parent directory of the package_config.json file, then you don't need to read package_graph.json.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"dart test --version" doesn't work in workspace folders
4 participants