Make MockBuilder support build_extensions option.

[LuisDuarte1]

This PR changes the behaviour of the buildMocks factory, in order to support the build_extensions option.

This is useful in order to change the destination directory of the mocks, allowing for more customizability as in this article.

This change is a bit opinionated, because it forces the end-user to have a output filename pattern that ends with .mocks.dart in order to be consistent with the already generated files from mockito. It also forces the user, to have an input pattern ending with .dart.

Closes #545

---

• I’ve reviewed the contributor guide and applied the relevant portions to this PR.

Contribution guidelines:

• See our contributor guide for general expectations for PRs.
• Larger or significant changes should be discussed in an issue before creating a PR.
• Contributions to our repos should follow the Dart style guide and use dart format.
• Most changes should add an entry to the changelog and may need to rev the pubspec package version.
• Changes to packages require corresponding tests.

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

[yanok]

LGTM, but we need:

1. Some documentation update to mention this feature, maybe even a FAQ entry;
2. Maybe add build_extensions to example/ project?

[yanok]

Corr: actually example/ is not a separate project, so maybe just change the top-level build.yaml file to show how it works?

[LuisDuarte1]

LGTM, but we need:

1. Some documentation update to mention this feature, maybe even a FAQ entry;
2. Maybe add build_extensions to example/ project?

I agree so I've added them.

Also, I found some edge cases with my implementation so I took care of them, I'll list them:

• Conflicting outputs, the user can supply multiple build_extensions and the builder will throw if it finds a conflicting output
• If the user provided a build_extension that doesn't match all .dart files that can be processed, the non-matched files would be ignored
• Relative includes in the .mocks.dart weren't being properly resolved if the output was in a different directory.

[yanok]

Thanks!

[yanok]

Could you please accept my changes and squash all commits into one?

[natebosch]

and squash all commits into one?

We should be able to squash for the internal commit - generally it's easier to avoid force-pushing over shared history in a PR.

[LuisDuarte1]

and squash all commits into one?

We should be able to squash for the internal commit - generally it's easier to avoid force-pushing over shared history in a PR.

Well didn't read that in time, sorry ^^ should I revert it then?

[natebosch]

should I revert it then?

No need - we can work with it either way. I was just hoping to inform on best practices since I frequently use shared PR history when reviewing in dart-lang reps 😄

[natebosch]

I don't understand this - user defined configuration for the .dart input will override the default. I would expect that the allowed outputs should always have a single value.

[natebosch]

Oh I see, the examples are using a ^test in the build extensions to substitute for a generate_for. cc @jakemac53 - did we predict this would happen when we enhanced the build extension syntax?

Maybe it would be better to only do a full override and not do a merge.

  @override
  final Map<String, List<String>> buildExtensions;

  MockBuilder({Map<String, List<String>>? buildExtensions})
      : buildExtensions = buildExtensions ??
            const {
              '.dart': ['.mocks.dart'],
            };

[LuisDuarte1]

Having the commit history would be helpful now ahah, but that was my first implementation, but there's a reason I changed it into a merge. I've discovered this when making the example for this config, example/build_extensions, when doing the full override and only having an input pattern that only partially covers all .dart files, the unmatched .dart files wouldn't be processed, even though there's a generate_for for those files. So my fix was to merge the user rules with the generic one ( .dart=>.mocks.dart)

[natebosch]

when doing the full override and only having an input pattern that only partially covers all .dart files, the unmatched .dart files wouldn't be processed, even though there's a generate_for for those files.

I think not processing them is preferable to silently falling back on potentially unexpected behavior. I think it is reasonable to expect the overridden config to cover all inputs.

[LuisDuarte1]

I think not processing them is preferable to silently falling back on potentially unexpected behavior. I think it is reasonable to expect the overridden config to cover all inputs.

I can understand your perspective, in this case it's best to be consistent and not introduce any unexpected behavior, even though I think my approach is more ergonomic for developers. I'll try to change it then.

[yanok]

We should be able to squash for the internal commit - generally it's easier to avoid force-pushing over shared history in a PR.

Well, actually Copybara already does the squash, so it's mostly about getting a nicer CL description, as now it will list all the commits. We should probably just fix our Copybara config to copy the PR description into CL.

[yanok]

We need to wait for the resolution of the discussion above. Also please convert the added double-quotes in the test file into single quotes: we have a lint complaining about that internally.

[yanok]

Now the formatter complains :) Could you please re-format it?

[LuisDuarte1]

Now the formatter complains :) Could you please re-format it?

Done. It seems that Dart 3.0 has a different formatting than Dart 2.19, a quick setup in github codespaces allowed me to format the files in the CI version 😅.

[yanok]

Done. It seems that Dart 3.0 has a different formatting than Dart 2.19, a quick setup in github codespaces allowed me to format the files in the CI version 😅.

Arghh, I see. Yes, I think we did change the formatter some time ago. Damn, then we should have just bumped the required SDK version I guess... I wonder if now formatter will start complaining internally, since we use more or less HEAD version... But let's wait.

[yanok]

Nope, it worked fine, thanks!