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

add use_test_throws_matchers #2631

Merged
merged 2 commits into from May 12, 2021
Merged

Conversation

a14n
Copy link
Contributor

@a14n a14n commented May 11, 2021

Use the throwsA matcher instead of try-catch with fail().

BAD:

// sync code
try {
  someSyncFunctionThatThrows();
  fail('expected Error');
} on Error catch (error) {
  expect(error.message, contains('some message'));
}
// async code
try {
  await someAsyncFunctionThatThrows();
  fail('expected Error');
} on Error catch (error) {
  expect(error.message, contains('some message'));
}

GOOD:

// sync code
expect(
  () => someSyncFunctionThatThrows(),
  throwsA(isA<Error>().having((Error error) => error.message, 'message', contains('some message'))),
);
// async code
await expectLater(
  () => someAsyncFunctionThatThrows(),
  throwsA(isA<Error>().having((Error error) => error.message, 'message', contains('some message'))),
);

@google-cla google-cla bot added the cla: yes label May 11, 2021
@coveralls
Copy link

coveralls commented May 11, 2021

Coverage Status

Coverage decreased (-0.3%) to 94.084% when pulling 8849c1f on a14n:use_test_throws_matchers into bf8de84 on dart-lang:master.

lib/src/rules/use_test_throws_matchers.dart Show resolved Hide resolved

if (isTestInvocation(lastBodyStatement, 'fail') &&
node.finallyBlock == null) {
rule.reportLint(node);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we could highlight the name 'fail' rather than the whole try statement.

@pq pq merged commit a4b5abd into dart-lang:master May 12, 2021
@a14n a14n deleted the use_test_throws_matchers branch May 12, 2021 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants