Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[e2e] Fix incorrect test results when one test passes then another fails #2866

Merged
merged 3 commits into from
Jul 25, 2020

Conversation

jiahaog
Copy link
Member

@jiahaog jiahaog commented Jul 8, 2020

Description

For example, the following test will result in an error reported for the first test case.

void main() {
  testWidgets('a test that passes', (tester) async {
    expect(true, true);
  });

  testWidgets('a test that fails', (tester) async {
    expect(true, false);
  });
}

We need to reset reportTestException back to the previous value after completion of runTest, or repeated failures will cause the exception handler for a previous test to be invoked, as they "stack". Instead of reseting it, however, do this once in the constructor because the test description is already provided by the function signature.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • No, this is not a breaking change.

For example, the following test will result in an error reported for the first test case.

```
void main() {
  testWidgets('a test that passes', (tester) async {
    expect(true, true);
  });

  testWidgets('a test that fails', (tester) async {
    expect(true, false);
  });
}
```

We need to reset `reportTestException` back to the previous value after completion of `runTest`, or repeated failures will cause the exception handler for a previous test to be invoked, as they "stack". Instead of reseting it, however, do this once in the constructor because the test description is already provided by the function signature.
@jiahaog jiahaog changed the title [e2e] Fix overly verbose stack traces in test failures [e2e] Fix incorrect test results when one test passes then another fails Jul 23, 2020
@jiahaog jiahaog marked this pull request as ready for review July 23, 2020 07:11
@jiahaog jiahaog requested a review from digiter as a code owner July 23, 2020 07:11
@jiahaog jiahaog requested a review from dnfield July 23, 2020 07:12
Copy link
Contributor

@dnfield dnfield left a comment

Choose a reason for hiding this comment

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

I think the change makes sense, but could you please add a test that would have failed without this?

I realize this might require actually running some tests and parsing the output.

@dnfield
Copy link
Contributor

dnfield commented Jul 23, 2020

Going to land this on the understanding that CI is red because of issues upstream in Flutter that have been resolved.

@dnfield
Copy link
Contributor

dnfield commented Jul 23, 2020

Sorry, commented on wrong PR...

@jiahaog
Copy link
Member Author

jiahaog commented Jul 24, 2020

Done, but this approach does slow down the tests significantly as it has to start up the flutter tester for each test in a child process. One thing I'm not sure of is assuming flutter to be in $PATH which might not work internally. Getting the tests running there is on my todo list and might be something to revisit in the future.

@jiahaog jiahaog requested a review from dnfield July 24, 2020 15:13
@jiahaog jiahaog merged commit 14608ca into flutter:master Jul 25, 2020
@jiahaog jiahaog deleted the e2e-stack-trace branch July 25, 2020 01:46
jarrodcolburn pushed a commit to jarrodcolburn/plugins that referenced this pull request Aug 20, 2020
…ils (flutter#2866)

* [e2e] Fix incorrect test results when one test passes then another fails

For example, the following test will result in an error reported for the first test case.

```
void main() {
  testWidgets('a test that passes', (tester) async {
    expect(true, true);
  });

  testWidgets('a test that fails', (tester) async {
    expect(true, false);
  });
}
```

We need to reset `reportTestException` back to the previous value after completion of `runTest`, or repeated failures will cause the exception handler for a previous test to be invoked, as they "stack". Instead of reseting it, however, do this once in the constructor because the test description is already provided by the function signature.

* Add a mechanism for testing test results
jorgefspereira pushed a commit to jorgefspereira/plugins_flutter that referenced this pull request Oct 10, 2020
…ils (flutter#2866)

* [e2e] Fix incorrect test results when one test passes then another fails

For example, the following test will result in an error reported for the first test case.

```
void main() {
  testWidgets('a test that passes', (tester) async {
    expect(true, true);
  });

  testWidgets('a test that fails', (tester) async {
    expect(true, false);
  });
}
```

We need to reset `reportTestException` back to the previous value after completion of `runTest`, or repeated failures will cause the exception handler for a previous test to be invoked, as they "stack". Instead of reseting it, however, do this once in the constructor because the test description is already provided by the function signature.

* Add a mechanism for testing test results
FlutterSu pushed a commit to FlutterSu/flutter-plugins that referenced this pull request Nov 20, 2020
…ils (flutter#2866)

* [e2e] Fix incorrect test results when one test passes then another fails

For example, the following test will result in an error reported for the first test case.

```
void main() {
  testWidgets('a test that passes', (tester) async {
    expect(true, true);
  });

  testWidgets('a test that fails', (tester) async {
    expect(true, false);
  });
}
```

We need to reset `reportTestException` back to the previous value after completion of `runTest`, or repeated failures will cause the exception handler for a previous test to be invoked, as they "stack". Instead of reseting it, however, do this once in the constructor because the test description is already provided by the function signature.

* Add a mechanism for testing test results
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants