feat(test): enhance logs and final test report in case of failing tests #563

agacemi · 2022-11-11T22:34:40Z

Description

In our company, we use very_good client for test. Log clarty are importants to identify quickly how many tests fails and why.

In addition the two other PRs (enabling ansi color #534, and fix multiline test name #535 ), this PR aims to enhance the three fellowing aspects of very_good test logs.

Final report in case of failed tests

Currently, very_good_test shows test file paths.

counter_cubit_counter_cubit_test_dart CounterCubit decrement emits [42] when state is 43 /Users/abdelaziz_gacemi/Devhome/GitProjects/opensource/bloc/examples/flutter_counter/test/.test_runner.dart (FAILED)
00:03 +9 -6: Some tests failed.
Failing Tests:
 - [FAILED] test/counter/cubit/counter_cubit_test.dart:8:5
 - [FAILED] test/counter/cubit/counter_cubit_test.dart:13:7
 - [FAILED] package:bloc_test/src/bloc_test.dart:155:5
 - [FAILED] package:bloc_test/src/bloc_test.dart:155:5
 - [FAILED] package:bloc_test/src/bloc_test.dart:155:5
 - [FAILED] package:bloc_test/src/bloc_test.dart:155:5

This PRs, we list failing tests grouped by test file name. We think that's better for developer experience.

CounterCubit decrement emits [42] when state is 43 /Users/abdelaziz_gacemi/Devhome/GitProjects/opensource/bloc/examples/flutter_counter/test/counter/cubit/counter_cubit_test.dart (FAILED)
00:03 +9 -6: Some tests failed.
Failing Tests:
 - /Users/abdelaziz_gacemi/Devhome/GitProjects/opensource/bloc/examples/flutter_counter/test/counter/cubit/counter_cubit_test.dart 
        - [FAILED] initial state is 2
        - [FAILED] CounterCubit initial state is 0
        - [FAILED] CounterCubit increment emits [42] when state is 41
        - [FAILED] CounterCubit decrement emits [-1] when state is 0
        - [FAILED] CounterCubit decrement emits [-1, -2] when state is 0 and invoked twice
        - [FAILED] CounterCubit decrement emits [42] when state is 43

More comprehensible logs even optimisation is applied

When optimization is applied, very_good generates and uses .test_runner.test. Log show this detail of implementation and make logs less clear and file link that reference .test_runner.dart useless because this file is removed at the end of execution.

In this PR, we hide this detail of implementaiton. Logs are the same with or without optimization.

Fix log in case of blocTest and more generally if exception is thrown from a zone.

Current algorithm extracts the test class path from the exception trace. This choice has a limitation in case of exception thrown inside a zone. For this reason, in case of failed test using blocTest, the test file name will be wrong and will be always - [FAILED] package:bloc_test/src/bloc_test.dart:155:5

In this PR, we fix this issue.

Type of Change

✨ New feature (non-breaking change which adds functionality)
🛠️ Bug fix (non-breaking change which fixes an issue)
❌ Breaking change (fix or feature that would cause existing functionality to change)
🧹 Code refactor
✅ Build configuration change
📝 Documentation
🗑️ Chore

renancaraujo

Hey @agacemi, thanks for this great contribution.

Everything looks good to me.
One question to @felangel: this is based on naming the top groups in the .test_runner file
to be the actual unchanged path of the test files, not snaked cases as they are now. Would that be a problem?

The bundle is out dated; would you mind updating it @agacemi?

lib/src/cli/flutter_cli.dart

lib/src/cli/cli.dart

agacemi · 2023-02-08T04:58:33Z

@renancaraujo as this PR modifies .test_runner_bundle.dart, could you indicate me the cleaner way to modify it.

renancaraujo · 2023-02-13T13:29:56Z

@renancaraujo, as this PR modifies .test_runner_bundle.dart, could you indicate to me the cleaner way to modify it?

Now that we have an unbundled brick in the codebase, the changes should be made on it, and then you could run tool/generate_test_optimizer_bundle.sh to bundle your changes

agacemi · 2023-02-16T10:51:05Z

@renancaraujo Rebase done. It's right, after brick unbundeling, it's more easier

wolfenrain · 2023-02-16T11:31:13Z

test/src/cli/flutter_cli_test.dart

@@ -459,7 +467,8 @@ void main() {
                  'test/counter/cubit/counter_cubit_test.dart 16:7     main.<fn>.<fn>\n',
              '\x1B[2K\rCounterCubit initial state is 0 /my_app/test/counter/cubit/counter_cubit_test.dart (FAILED)',
              '\x1B[2K\rFailing Tests:\n'
-                  '\x1B[2K\r - [FAILED] test/counter/cubit/counter_cubit_test.dart:16:7\n'
+                  '\x1B[2K\r - /my_app/test/counter/cubit/counter_cubit_test.dart \n'


Does this / mean that it will always show the absolute path, I wonder if we can get the relative path based on the current directory. That would make the test output cleaner while keeping the info still useful

Yes, in the final report, we show always the absolute path and failing tests by each file. My purpose is that the final report keeps the same format in the log before the summary (in the log, full path is used). Paths are clickable from terminal inside IDE to open it. However, I agree that is information is less useful in case of log on CI.

I looked in the code how I can get only relative path. I didn't find a reliable approach because we can have also recursive excution via "--recursive" flag.

We can just make it relative to the Directory.current, which will work for the --recursive flag as that runs from the current directory

Done using path.relative. Fixtures changed to be dynamic according to command working directory..
@renancaraujo After rebase, I lost your approval ^^

@wolfenrain @renancaraujo are there any other points to deal with?

wolfenrain · 2023-02-21T12:26:37Z

bricks/test_optimizer/__brick__/test/.test_optimizer.dart

@@ -10,7 +10,7 @@ import 'package:flutter_test/flutter_test.dart';
 {{/tests}}
 void main() {
 {{#isFlutter}}  goldenFileComparator = _TestOptimizationAwareGoldenFileComparator();{{/isFlutter}}
-{{#tests}}  group('{{#snakeCase}}{{{.}}}{{/snakeCase}}', () { {{#snakeCase}}{{{.}}}{{/snakeCase}}.main(); });
+{{#tests}}  group('{{{.}}}', () { {{#snakeCase}}{{{.}}}{{/snakeCase}}.main(); });


What is the reason for changing the group name to the path?

This could potentially fail if for some reason the path has a ' in it, then the string would not be valid.

In the algorithm that build the final report, I would like be able to construct the full file name and keep the initial file name with package. I use the same string that used from import.

As I explains in the PR description, my goal is that the report in using or without using optimization is the same. In fact, optimization is the implementation detail that seem to me useful to hide.

I can't find a example of problematic case

Here an example of what generated for counter example.

// GENERATED CODE - DO NOT MODIFY BY HAND // Consider adding this file to your .gitignore. import 'dart:io'; import 'package:flutter_test/flutter_test.dart'; import 'app_test.dart' as app_test_dart; import 'counter/cubit/counter_cubit_test.dart' as counter_cubit_counter_cubit_test_dart; import 'counter/view/counter_page_test.dart' as counter_view_counter_page_test_dart; import 'counter/view/counter_view_test.dart' as counter_view_counter_view_test_dart; void main() { goldenFileComparator = _TestOptimizationAwareGoldenFileComparator(); group('app_test.dart', () { app_test_dart.main(); }); group('counter/cubit/counter_cubit_test.dart', () { counter_cubit_counter_cubit_test_dart.main(); }); group('counter/view/counter_page_test.dart', () { counter_view_counter_page_test_dart.main(); }); group('counter/view/counter_view_test.dart', () { counter_view_counter_view_test_dart.main(); }); } class _TestOptimizationAwareGoldenFileComparator extends LocalFileComparator { final List<String> goldenFilePaths; _TestOptimizationAwareGoldenFileComparator() : goldenFilePaths = _goldenFilePaths, super(_testFile); static Uri get _testFile { final basedir = (goldenFileComparator as LocalFileComparator).basedir.toString(); return Uri.parse("$basedir/.test_optimizer.dart"); } static List<String> get _goldenFilePaths => Directory.fromUri((goldenFileComparator as LocalFileComparator).basedir) .listSync(recursive: true, followLinks: true) .whereType<File>() .map((file) => file.path) .where((path) => path.endsWith('.png')) .toList(); @override Uri getTestUri(Uri key, int? version) { final keyString = key.path; return Uri.parse(goldenFilePaths .singleWhere((goldenFilePath) => goldenFilePath.endsWith(keyString))); } }

Can we exclude this change from this PR as it's not directly related and is susceptible to issues as @wolfenrain pointed out?

@felangel In my opinion it's related. Otherwise, the final report is less clear. Below an example of report if I exclude this change. The file name will be counter_cubit_counter_cubit_test_dart

CounterCubit initial state is 0 /Users/abdelaziz_gacemi/Devhome/GitProjects/opensource/bloc/examples/flutter_counter/test/counter_cubit_counter_cubit_test_dart (FAILED) 00:02 +13 -1: Some tests failed. Failing Tests: - test/counter_cubit_counter_cubit_test_dart - [FAILED] CounterCubit initial state is 0

The template '{{{.}}}' is already used for generate import line.

import 'counter/cubit/counter_cubit_test.dart' as counter_cubit_counter_cubit_test_dart; group('counter/cubit/counter_cubit_test.dart', () { counter_cubit_counter_cubit_test_dart.main();

@felangel @wolfenrain I rebased from main branch to get the last commit to deal with special character.. I made some tests with a file name that contains ' like widget'_special_test.dart. Below, results:

withflutter test : KO

with very_good test in main branch: KO

like main branch, on my branch is KO

The generated .test_optimizer.dat look like this:

import 'widget[1]_special_test.dart' as _a; import 'widget\'_special_test.dart' as _b; import 'widget_test.dart' as _c; void main() { goldenFileComparator = _TestOptimizationAwareGoldenFileComparator(); group('widget[1]_special_test.dart', () { _a.main(); }); group('widget\'_special_test.dart', () { _b.main(); }); group('widget_test.dart', () { _c.main(); }); }

and the obtained error look like:

test/.test_optimizer.dart:10:8: Error: Error when reading 'test/widget': No such file or directory import 'widget'_special_test.dart' as _b; ^ test/.test_optimizer.dart:16:48: Error: Undefined name '_b'. group('widget'_special_test_dart', () { _b.main(); }); ^^

Could we exclude this special use case from the scope of this PR as this case is also problematic case for flutter test and very_good test on main branch

Thank you

I am fine with keeping it like it is then if we are already using this and therefore having this issue.

What do you think @renancaraujo?

agacemi requested review from felangel, wolfenrain and renancaraujo as code owners November 11, 2022 22:34

agacemi force-pushed the enhance-report branch from 86fbe90 to 8ea4910 Compare November 12, 2022 06:22

renancaraujo self-assigned this Nov 14, 2022

renancaraujo reviewed Dec 12, 2022

View reviewed changes

lib/src/cli/flutter_cli.dart Outdated Show resolved Hide resolved

lib/src/cli/flutter_cli.dart Outdated Show resolved Hide resolved

lib/src/cli/flutter_cli.dart Outdated Show resolved Hide resolved

renancaraujo added the waiting for response Waiting for follow up label Dec 12, 2022

agacemi force-pushed the enhance-report branch 2 times, most recently from 3730d93 to d2681cb Compare December 19, 2022 15:07

renancaraujo self-requested a review January 2, 2023 13:43

renancaraujo removed the waiting for response Waiting for follow up label Jan 2, 2023

renancaraujo previously approved these changes Jan 19, 2023

View reviewed changes

lib/src/cli/cli.dart Show resolved Hide resolved

renancaraujo self-requested a review January 19, 2023 20:21

renancaraujo added the waiting for response Waiting for follow up label Jan 23, 2023

agacemi dismissed renancaraujo’s stale review via 7821cf3 February 8, 2023 04:35

agacemi force-pushed the enhance-report branch 2 times, most recently from 7821cf3 to 254ae59 Compare February 8, 2023 04:43

renancaraujo removed the waiting for response Waiting for follow up label Feb 13, 2023

agacemi force-pushed the enhance-report branch 2 times, most recently from d9fc5e5 to c211159 Compare February 16, 2023 10:40

renancaraujo previously approved these changes Feb 16, 2023

View reviewed changes

wolfenrain reviewed Feb 16, 2023

View reviewed changes

agacemi dismissed renancaraujo’s stale review via f6f4147 February 17, 2023 15:26

agacemi force-pushed the enhance-report branch from f6f4147 to 1551929 Compare February 17, 2023 15:42

wolfenrain reviewed Feb 21, 2023

View reviewed changes

renancaraujo added the waiting for response Waiting for follow up label Feb 21, 2023

feat(test): enhance logs and final test report in case of failing tests

d1e9b4a

agacemi force-pushed the enhance-report branch from 1551929 to d1e9b4a Compare February 23, 2023 07:03

renancaraujo removed the waiting for response Waiting for follow up label Feb 27, 2023

renancaraujo removed their assignment Feb 27, 2023

agacemi mentioned this pull request Mar 1, 2023

feat(test): enable running very_good test --optimization for specific files/directory #564

Closed

7 tasks

wolfenrain previously approved these changes Mar 2, 2023

View reviewed changes

Merge branch 'main' into enhance-report

8e04ac9

renancaraujo dismissed wolfenrain’s stale review via 8e04ac9 March 7, 2023 19:50

renancaraujo requested a review from wolfenrain March 7, 2023 19:50

renancaraujo approved these changes Mar 7, 2023

View reviewed changes

wolfenrain approved these changes Mar 8, 2023

View reviewed changes

renancaraujo merged commit 3b5668e into VeryGoodOpenSource:main Mar 9, 2023

agacemi deleted the enhance-report branch April 10, 2023 08:39

renancaraujo mentioned this pull request Apr 24, 2023

spike: Wrong test suite identified when running "very_good test" with optimization #401

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(test): enhance logs and final test report in case of failing tests #563

feat(test): enhance logs and final test report in case of failing tests #563

agacemi commented Nov 11, 2022

renancaraujo left a comment

agacemi commented Feb 8, 2023

renancaraujo commented Feb 13, 2023 •

edited

Loading

agacemi commented Feb 16, 2023

wolfenrain Feb 16, 2023

agacemi Feb 16, 2023 •

edited

Loading

wolfenrain Feb 16, 2023

agacemi Feb 17, 2023

agacemi Feb 21, 2023

wolfenrain Feb 21, 2023

agacemi Feb 21, 2023 •

edited

Loading

felangel Feb 21, 2023

agacemi Feb 21, 2023 •

edited

Loading

agacemi Feb 23, 2023 •

edited

Loading

wolfenrain Mar 2, 2023

feat(test): enhance logs and final test report in case of failing tests #563

feat(test): enhance logs and final test report in case of failing tests #563

Conversation

agacemi commented Nov 11, 2022

Description

Final report in case of failed tests

More comprehensible logs even optimisation is applied

Fix log in case of blocTest and more generally if exception is thrown from a zone.

Type of Change

renancaraujo left a comment

Choose a reason for hiding this comment

agacemi commented Feb 8, 2023

renancaraujo commented Feb 13, 2023 • edited Loading

agacemi commented Feb 16, 2023

wolfenrain Feb 16, 2023

Choose a reason for hiding this comment

agacemi Feb 16, 2023 • edited Loading

Choose a reason for hiding this comment

wolfenrain Feb 16, 2023

Choose a reason for hiding this comment

agacemi Feb 17, 2023

Choose a reason for hiding this comment

agacemi Feb 21, 2023

Choose a reason for hiding this comment

wolfenrain Feb 21, 2023

Choose a reason for hiding this comment

agacemi Feb 21, 2023 • edited Loading

Choose a reason for hiding this comment

felangel Feb 21, 2023

Choose a reason for hiding this comment

agacemi Feb 21, 2023 • edited Loading

Choose a reason for hiding this comment

agacemi Feb 23, 2023 • edited Loading

Choose a reason for hiding this comment

wolfenrain Mar 2, 2023

Choose a reason for hiding this comment

renancaraujo commented Feb 13, 2023 •

edited

Loading

agacemi Feb 16, 2023 •

edited

Loading

agacemi Feb 21, 2023 •

edited

Loading

agacemi Feb 21, 2023 •

edited

Loading

agacemi Feb 23, 2023 •

edited

Loading