Add option to override whole provider/argprovider

[manuel-plavsic]

This PR attempts to solve #35.

Tl;dr: I propose to remove overrideWithValue in favour of overrideWithProvider. For example, instead of:

  testWidgets('''ProviderScopeOverride should override providers''', (
    tester,
  ) async {
    final numberProvider = Provider<int>((_) => 0);
    await tester.pumpWidget(
      ProviderScopeOverride(
        overrides: [
          numberProvider.overrideWithValue(100),
        ],
        child: MaterialApp(
          home: ProviderScope(
            providers: [
              numberProvider,
            ],
            child: Builder(
              builder: (context) {
                final number = numberProvider.of(context);
                return Text(number.toString());
              },
            ),
          ),
        ),
      ),
    );
    expect(find.text('100'), findsOneWidget);
  });

one should write

  testWidgets('''ProviderScopeOverride should override providers''', (
    tester,
  ) async {
    final numberProvider = Provider<int>((_) => 0);
    final mockNumberProvider = Provider<int>((_) => 100);
    await tester.pumpWidget(
      ProviderScopeOverride(
        overrides: [
          numberProvider.overrideWithProvider(mockNumberProvider),
        ],
        child: MaterialApp(
          home: ProviderScope(
            providers: [
              numberProvider,
            ],
            child: Builder(
              builder: (context) {
                final number = numberProvider.of(context);
                return Text(number.toString());
              },
            ),
          ),
        ),
      ),
    );
    expect(find.text('9'), findsOneWidget);
  });

Explanation: this second variant can do everything the first variant can do. It is just more powerful, as also lazy, a proper create and dispose can be specified.

This is even more useful when working with argument providers, whose mocking functionality is currently not ideal IMO. At the moment, one can only override the whole argument provider, but cannot make argument distinctions. This defeats the point of argument providers. Here is how it currently is on main:

disco/packages/disco/test/disco_test.dart

Lines 728 to 753 in 738b1ea

	testWidgets('''ProviderScopeOverride should override argument providers''', (
	tester,
	) async {
	final numberProvider = Provider.withArgument((_, int arg) => arg);
	await tester.pumpWidget(
	ProviderScopeOverride(
	overrides: [
	numberProvider.overrideWithValue(16),
	],
	child: MaterialApp(
	home: ProviderScope(
	providers: [
	numberProvider(1),
	],
	child: Builder(
	builder: (context) {
	final number = numberProvider.of(context);
	return Text(number.toString());
	},
	),
	),
	),
	),
	);
	expect(find.text('16'), findsOneWidget);
	});

So, we definitely need arguments. In my current implementation, they would work like this:

  testWidgets('''ProviderScopeOverride should override argument providers''', (
    tester,
  ) async {
    final numberProvider = Provider.withArgument((_, int arg) => arg);
    await tester.pumpWidget(
      ProviderScopeOverride(
        overrides: [
          numberProvider.overrideWithValue(1, 16), // 1 is the arg, 16 the value
        ],
        child: MaterialApp(
          home: ProviderScope(
            providers: [
              numberProvider(1),
            ],
            child: Builder(
              builder: (context) {
                final number = numberProvider.of(context);
                return Text(number.toString());
              },
            ),
          ),
        ),
      ),
    );
    expect(find.text('16'), findsOneWidget);
  });

However, I don't think this approach is ideal. People will have to write some custom logic somewhere every time they want to use the override (because they need to use specific arguments and provide specific values for that).

We should purse this approach instead:

  testWidgets('''ProviderScopeOverride should override argument providers''', (
    tester,
  ) async {
    final numberProvider = Provider.withArgument((_, int arg) => arg);
    final mockNumberProvider = Provider.withArgument((_, int arg) => 16);
    await tester.pumpWidget(
      ProviderScopeOverride(
        overrides: [
          numberProvider.overrideWithProvider(mockNumberProvider),
        ],
        child: MaterialApp(
          home: ProviderScope(
            providers: [
              numberProvider(1),
            ],
            child: Builder(
              builder: (context) {
                final number = numberProvider.of(context);
                return Text(number.toString());
              },
            ),
          ),
        ),
      ),
    );
    expect(find.text('16'), findsOneWidget);
  });

This way, one can just write the custom logic in mockNumberProvider's create and dispose, without polluting the widget tree. This is also what was asked in the linked issue.

The logic for this would still have to be adapted in ProviderScope. Before I invest time there, is this okay for you @nank1ro?
Right now, I added sealed classes and pattern matching to allow both overrideWithValue and overrideWithProvider, but if we get rid of the former, we don't need the sealed classes and pattern matching anymore.

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4d25b691-842a-4851-a488-3a145c1a48d2

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/allow-whole-provider-override

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

@@            Coverage Diff            @@
##              main       #40   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           10        10           
  Lines          288       288           
=========================================
  Hits           288       288           

Files with missing lines	Coverage Δ
...c/models/overrides/provider_argument_override.dart	100.00% <100.00%> (ø)
...co/lib/src/models/overrides/provider_override.dart	100.00% <100.00%> (ø)
...kages/disco/lib/src/models/providers/provider.dart	100.00% <100.00%> (ø)
...co/lib/src/models/providers/provider_argument.dart	100.00% <100.00%> (ø)
packages/disco/lib/src/widgets/provider_scope.dart	100.00% <100.00%> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[manuel-plavsic]

Technically, we could easily add two wrapper types MockProvider and MockArgProvider (with constructor MockProvider.withArg) to ensure only mocks can be specified in ProviderScopeOverride.overrides and only normal providers can specified in ProviderScope.providers. This would prevent using the wrong kind of provider in the wrong widget.

[nank1ro]

@manuel-plavsic If you remove overrideWithValue it's going to be a breaking change and we have to bump a major.
I'd prefer to mark it as deprecated and remove it later in a next major.
So existing users can migrate slowly.

Additionally, what do you think about

numberProvider.overrideWith(mockNumberProvider),
// instead of 
numberProvider.overrideWithProvider(mockNumberProvider),

?

[manuel-plavsic]

@nank1ro good idea, I'll rename the method to overrideWith 😉

[manuel-plavsic]

and yes, it makes sense to keep overrideWithValue and mark it as deprecated