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

Warn when tests are accidentally disabled due to nesting within setUp() method #1961

Open
rtannenbaum opened this issue Feb 28, 2023 · 3 comments · May be fixed by #2012
Open

Warn when tests are accidentally disabled due to nesting within setUp() method #1961

rtannenbaum opened this issue Feb 28, 2023 · 3 comments · May be fixed by #2012
Labels
type-enhancement A request for a change that isn't a bug

Comments

@rtannenbaum
Copy link

Hello,

We recently discovered that several tests were accidentally nested inside a setUp() method.

While the tests compile, they were not actually run by the test runner, and we had no way to detect this. We only happened to notice coincidentally.

It would be helpful if the test runner could warn that tests are disabled when this happens.

Thanks for your consideration.

Rachel

@rtannenbaum rtannenbaum added the type-enhancement A request for a change that isn't a bug label Feb 28, 2023
@jakemac53
Copy link
Contributor

jakemac53 commented Feb 28, 2023

Normally we would error for any calls to test that are not directly created inside main (or a group in main).

However it looks like if you have a setUp but no actual tests, then we never run the setUp and therefore never throw the expected error:

import 'package:test/test.dart';

void main() {
  setUp(() {
    test('hello', () {
      expect(1, 2);
    });
  });
}

The above for example does not fail, but if you add a real test after the call to setUp, it will fail.

@jakemac53
Copy link
Contributor

From a practical standpoint I am not sure how to really resolve this, we don't want to run all setUp functions in a file if no tests are being ran in that file (maybe they were skipped or were not expected to run on the current platform etc).

@jakemac53
Copy link
Contributor

One possible fix could be to warn if we see any group which has no tests defined (transitively).

That should catch most cases as long as any of those tests are actually ran in some configuration.

natebosch added a commit that referenced this issue May 10, 2023
Closes #1961

After declaring a group check for declared tests, if there were none add
a synthetic test that fails with an error about the empty group.
@natebosch natebosch linked a pull request May 10, 2023 that will close this issue
natebosch added a commit to dart-lang/yaml that referenced this issue May 10, 2023
These tests had been commented out since the original commit in this
repo even though most of them would pass.

With these tests commented out the suite has an empty `group` which may
become disallowed.
dart-lang/test#1961

Change double quotes to single quotes.

Skip a test which is currently failing due to an error parsing a float
value with a single digit.
jonasfj pushed a commit to dart-lang/yaml that referenced this issue May 10, 2023
These tests had been commented out since the original commit in this
repo even though most of them would pass.

With these tests commented out the suite has an empty `group` which may
become disallowed.
dart-lang/test#1961

Change double quotes to single quotes.

Skip a test which is currently failing due to an error parsing a float
value with a single digit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants