rrousselGit / state_notifier

ValueNotifier, but outside Flutter and with some extra perks
MIT License
311 stars 28 forks source link

Test failures swallowed in addListener callback #20

Closed frank06 closed 4 years ago

frank06 commented 4 years ago

I am testing stuff inside of the addListener callback, but since errors are caught and "redirected" to onError, test failures are never exposed:

    notifier.addListener(
      expectAsync1((model) {
        if (i == 0) expect(model, matcher);
        if (i == 1) expect(model, matcher);
        i++;
      }, count: 2),
      fireImmediately: false,
    );

Any of those expects failing, and the test will still pass.

I tried different ways of overriding the callback but none report back the failure:

    notifier.onError = (err, stack) {
      throw err;
      // print(err);
      // throw TestFailure(err.toString());
    };

So my first workaround is checking on the counter at the end of the test:

expect(i, equals(2));

Any better ideas or workarounds?

rrousselGit commented 4 years ago

Hum onError could default to Zone.current.handleUncaughtError

frank06 commented 4 years ago
notifier.onError = Zone.current.handleUncaughtError;

does indeed work. Thank you Remi for the speedy response and the black magic!

rrousselGit commented 4 years ago

I'll make a pr that makes it the default

frank06 commented 4 years ago

@rrousselGit would it be possible to just throw error? Debugging errors inside listeners is a pain

rrousselGit commented 4 years ago

What do you mean by pain?

frank06 commented 4 years ago

There was an exception thrown inside a listener and there was no way to trace back to the line that caused it. Debugger jumped to the state_notifier package line with throw Error(), arguments error and stackTrace are not helpful.

Using 0.5.0

frank06 commented 4 years ago

Why not throw the error directly?

try {
        listenerEntry.listener(value);
      } catch (error, stackTrace) {
        if (onError != null) {
          onError(error, stackTrace);
        } else {
          throw error;
        }
      }
rrousselGit commented 4 years ago

It notifies all listeners first. Otherwise some listeners could be left out