[New rule] missing_test_assertion

[orevial]

Please describe what the rule should do:

To be useful, a test should make an assertion, expect something. A test that only initializes things but make no assertion afterwards should trigger a warning to tell that "the test contains no assertion". The developer might have just forgot the expectation at the end. However this is dangerous because :

• The test is useless (apart from validating that the initialization of objects / widgets / whatever causes no exception hence no test failure)
• It is misleading because there is still a block of code and developers will potentially think that a piece of code is tested when it's really not. Same goes for code coverage, a flaky test with no assertion will still contribute to the overall code coverage even if it's not actually testing anything

As I never implemented a linter rules I will leave some notes below :

• is there any scenario where having no test assertions is actually a valid test case ?
• for the rule to be powerful it should also make sure that any assertion from an external package (e.g. mockito/mocktail verify, alchemist testGoldens, etc) is valid too... is that a good idea ?
• can we be exhaustive with the kinds of assertions the rule supports ?

I will happily discuss these questions !

What category of rule is this? (place an "X" next to just one item)

• Warns about a potential error (problem)
• Suggests an alternate way of doing something (suggestion)
• Other (please specify:)

Provide 2-3 code examples that this rule will warn about (it will be better if you can provide both good and bad examples):

❌ Bad:

test('bad unit test', () {
    // Given
    final a = 1;
    final b = 2;

    // When
    final c = a + 1;
});

✅ Good:

test('good unit test', () {
    // Given
    final a = 1;
    final b = 2;

    // When
    final c = a + 1;

    // Then : actual assertion
    expect(b, c);
});

❌ Bad:

testWidgets('bad widget test', (WidgetTester tester) async {
    await tester.pumpWidget(MyApp());
    await tester.pumpAndSettle();
});

✅ Good:

testWidgets('good widget test', (WidgetTester tester) async {
    await tester.pumpWidget(MyApp());
    await tester.pumpAndSettle();

    expect(find.text('Welcome'), findsOneWidget);
});

❌ Bad:

test('bad unit test', () {
    // Given (use either Mocktail or Mockito)
    final serviceA = ServiceA(MockServiceB());

    // When
    serviceA.computationThatUsesServiceB();
});

✅ Good:

test('bad unit test', () {
    // Given (use either Mocktail or Mockito)
    final serviceA = ServiceA(MockServiceB());

    // When
    serviceA.computationThatUsesServiceB();

    // Then
    verify(serviceB.compute());
});

Are you willing to submit a pull request to implement this rule?

Yes

[incendial]

@orevial amazing rule!

[lsaudon]

• Can't there be tests in which you don't want to expect?
• If we use third party libraries (shouldly), we will have false negatives?

[incendial]

The rule can still be ignored or we can support the new cases iteratively. But this rule looks useful, since it's easy to forget about expect, especially for newcomers.

[lsaudon]

I tried to implement it.
Unfortunately, I don't understand AST 😥😀

[incendial]

If you want to give it another shot, you need to find all test function invocations and check if they invoke expect. But the best way is to write test and check the AST there.

[lsaudon]

I can see the test function, but I can't see the code in the body parameter.

[incendial]

First, you need to define the function you're trying to call:

And then you need to visit invocations, not declarations and the callback will be one of the arguments:

[lsaudon]

I never managed to get the debugger like that, but I did with the screenshot.
Thanks a lot.