MethodChannel to mock ResultType.noAppToOpen in test

[jadensiow93]

Hello,

Currently I am trying to mock the ResultType.noAppToOpen in the test as shown below.

So I did the method channel below. const MethodChannel('open_file').setMockMethodCallHandler(methodCall) { return OpenResult(message: 'No APP found to open this file.' , type: ResultType.noAppToOpen);

But I am unable to have the desired return of ResultType.noAppToOpen in my test. Am I doing the methodChannel wrongly for this since there is filePath inside OpenFile.open(filePath) ? Or is the methodChannel not working for this. Thank you.

[ValentinVignal]

The issue is the you are not running your test on iOS or Android. So the condition https://github.com/crazecoder/open_file/blob/2cadda6d282935afce8b02aefe41a198f0c49c0c/lib/src/plaform/open_file.dart#L23

is true.

If you were running on iOS and Android, mocking the channel open_file would work because it is calling https://github.com/crazecoder/open_file/blob/2cadda6d282935afce8b02aefe41a198f0c49c0c/lib/src/plaform/open_file.dart#L57

at the end.

The issue here is that there is no way no mock whatever is inside the if of

https://github.com/crazecoder/open_file/blob/2cadda6d282935afce8b02aefe41a198f0c49c0c/lib/src/plaform/open_file.dart#L23

It would be nice if this package could include some test examples. I created an issue for that : https://github.com/crazecoder/open_file/issues/181

[burhanrashid52]

I think having an API where the target platform is checked by the BuildContext will help to replace the variant in testing using variant: param in testWidget. More here

Something like this. OpenFile.open(context,filePath);

[ValentinVignal]

Or and idea would be to not use static methods anymore as they are not mockable, you could do something like that:

class OpenFile {
  OpenFile._();

  static OpenFile _instance = OpenFile._();

  static OpenFile get instance => _instance;

  @visibleForTesting
  static OpenFile set instance(OpenFile instance) {
    // So we can give a mock in the tests
    _instance = instance;
  }
  
  // Not a static method anymore.
  Future<OpenResult> open(/* ... */) {/* ... */}  
}

And instead of do

OpenFile.open();

it will be

OpenFile.instance.open();

[burhanrashid52]

@ValentinVignal This is also a fair solution. But I don't think it's backward compatible and might break a lot of code if used extensively in the app. Something like this might work.

static Future<OpenResult> open(String filePath,{BuildContext context}) {
   final platform = context?.targetPlatform ?? getPlatformFromStaticCheck();
}  

[burhanrashid52]

@ValentinVignal This is also a fair solution. But I don't think it's backward compatible and might break a lot of code if used extensively in the app. Something like this might work.

static Future<OpenResult> open(String filePath,{BuildContext context}) {
   final platform = context?.targetPlatform ?? getPlatformFromStaticCheck();
}  

If @crazecoder is happy with the above solution then I can create a PR for it.

[ValentinVignal]

@burhanrashid52 Yes my suggestion is a breaking change, but if you increase the version number to 4.0.0 and mention it in the change logs it should be fine right ?

The issue your approach is that we would have to carry this context a bit everywhere, even in service classes/methods that are not supposed to use the BuildContext (not UI).

[burhanrashid52]

@burhanrashid52 Yes my suggestion is a breaking change, but if you increase the version number to 4.0.0 and mention it in the change logs it should be fine right ?

I will leave that up to the author of this library. However, I personally don't prefer OpenFile.instance pattern. Seems a lot verbose to me.

The issue your approach is that we would have to carry this context a bit everywhere, even in service classes/methods that are not supposed to use the BuildContext (not UI).

In the example, I share, BuildContext is optional. We can abstract out the platform check to a separate class, and based on parameters we can decide which implementation to use and use that abstract class throughout the library

[GP4cK]

I agree with https://github.com/crazecoder/open_file/issues/180#issuecomment-1044603096, I'd rather not have to pass around the context which is UI related in my services. If it comes at the cost of having the code a little more verbose, I'm willing to pay the price.

[burhanrashid52]

Hmm... Maybe using a static instance inside the static open method might solve both issues.

[ValentinVignal]

Hmm... Maybe using a static instance inside the static open method might solve both issues.

It would yes. But for it to be mocked, it would need to use a public method from the instance

[mixinDennisBarzanoffMixin]

just make another wrapper service and mock it, that would work regardless of how this library is written. I did this in my case