[`analyzer_testing`] Parameterizing Destination of `ResourceProviderMixin.newSinglePackageConfigJsonFile`

[RohitSaily]

ResourceProviderMixin of the analyzer_testing package has a method newSinglePackageConfigJsonFile which is used to generate a package config specifying one package as a dependency. Currently it generates a config for a package that lists itself as a dependency.

Parameterizing the destination can allow the package being depended on and the package the config is being made for to be distinct. This is useful in setting up a mock package that a package being tested needs as a dependency.

Not sure if it is a problem in the SDK per se - but the method and its documentation was a bit confusing since it seems like a generic method for creating a package config but only does so in the way described at the start of this post. I propose introducing the destination parameter so it can be used to create an arbitrary single-package config file.

Changes: https://dart-review.googlesource.com/c/sdk/+/450840

[bwilkerson]

The method ResourceProviderMixin. newSinglePackageConfigJsonFile is a convenience method. The method ResourceProviderMixin.newPackageConfigJsonFileFromBuilder is the more fully featured API. With the builder you can add as many packages as you want. Let me know if that doesn't meet your needs.

[srawlins]

Currently it generates a config for a package that lists itself as a dependency.

It needs to do at least this. Every package_config.json file for a package must include an entry for that package. Actually, even workspace package_config.json files include an entry for the top-level directory; it looks like pub picks some synthetic name.

[RohitSaily]

Thanks for the info. I understand the method now and its use as it is. This change would just expand on its convenience by allowing it to also be used to establish a dependency between a user package and mock package within a test. It's just a minor addition, not too important but useful nonetheless. This change retains the current behaviour but also makes it useful for the aforementioned use.

For example - I have the user package a and I want to set up a mock package b that a depends on because I am testing inter-package behaviour ie something to do with imports, doing look ups of another package, or managing multi-package data. This method can be repurposed to conveniently create the config file to establish the dependency when setting up the virtual test environment.

Its just an introduction of an additional convenience.

[bwilkerson]

... allowing it to also be used to establish a dependency between a user package and mock package within a test.

Actually, it's introducing a second way of accomplishing the same thing that newPackageConfigJsonFileFromBuilder accomplishes in a more general way.

And as Sam pointed out it breaks the existing behavior. Every package should have a binding to itself in the package_config.json file, even if it also has bindings for other packages.

I think the current design is better, even if it seems convenient for one specific use case.

[RohitSaily]

Actually, it's introducing a second way of accomplishing the same thing that newPackageConfigJsonFileFromBuilder accomplishes in a more general way.

I'm sorry I do not understand - I know the builder can be used for the use case I described but the same could be said for this convenience method. It's just that the method is more concise and direct.

And as Sam pointed out it breaks the existing behavior. Every package should have a binding to itself in the package_config.json file, even if it also has bindings for other packages.

I agree with this and I can now see why the config would need at least two packages listed in the config for the use case I mentioned, which would make it inconsistent with its name.

---

I am no longer in favour of making this change, so I will close the code submission.

Before I close this issue, I realized I did some tests with just the dependency package listed in the config file (the package does not list itself in the config file). Everything functioned as expected. Should there be some sort of error or lint to report this? Or are there reasons why this functions optionally.

[bwilkerson]

The package_config.json file is used to know how to resolve a package:packageName URI. If you don't have any such URIs in the code being tested then you won't notice if the entry for packageName is missing from the file. (Not sure that answers your question.)

[RohitSaily]

That does help me understand it better, thanks. I will close this issue now.