Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test failures swallowed in addListener callback #20

Closed
frank06 opened this issue Jun 4, 2020 · 8 comments
Closed

Test failures swallowed in addListener callback #20

frank06 opened this issue Jun 4, 2020 · 8 comments
Labels
enhancement New feature or request

Comments

@frank06
Copy link

frank06 commented Jun 4, 2020

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
Copy link
Owner

Hum onError could default to Zone.current.handleUncaughtError

@rrousselGit rrousselGit added the enhancement New feature or request label Jun 4, 2020
@frank06
Copy link
Author

frank06 commented Jun 4, 2020

notifier.onError = Zone.current.handleUncaughtError;

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

@rrousselGit
Copy link
Owner

I'll make a pr that makes it the default

@frank06
Copy link
Author

frank06 commented Jun 15, 2020

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

@rrousselGit
Copy link
Owner

What do you mean by pain?

@frank06
Copy link
Author

frank06 commented Jun 16, 2020

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
Copy link
Author

frank06 commented Jun 16, 2020

Why not throw the error directly?

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

@rrousselGit
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants