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

Test failures in tearDownAll are attributed to the correct line but wrong file #4681

Closed
DanTup opened this issue Aug 9, 2023 · 4 comments
Closed
Labels
in testing Relates to test execution of Dart/Flutter tests for end users is bug
Milestone

Comments

@DanTup
Copy link
Member

DanTup commented Aug 9, 2023

Moved from flutter/flutter#131681. I thought this was just a request for dart-lang/test#1851, however looking at the JSON we do get the location of the tearDownAll method, but we seem to record the line/col from that event in the file.

Here, "Go to Test" on "(tearDownAll)" is navigating to line 33 of the left file, instead of the right one. The test I ran was on line 26 of the left file.

image

The test event looks like:

 {
	"test": {
		"id": 5,
		"name": "(tearDownAll)",
		"suiteID": 0,
		"groupIDs": [
			2
		],
		"metadata": {
			"skip": false,
			"skipReason": null
		},
///////////////////////////////////////// 👇 This info looks correct 
		"line": 33,
		"column": 3,
		"url": "file:///C:/Dev/Google/leak_tracker/pkgs/leak_tracker_flutter_test/test/tests/end_to_end/leak_tracking_config_test/flutter_test_config.dart"
	},
	"type": "testStart",
	"time": 1072
},
@DanTup
Copy link
Member Author

DanTup commented Aug 9, 2023

With fix:

tests.mp4

@DanTup
Copy link
Member Author

DanTup commented Aug 9, 2023

Turns out this will only work in Flutter because of testExecutable because it allows the setUpAll and tearDownAll functions to be called without the test suite file in the stack.

If you trigger calling setUpAll() and tearDownAll() from inside the suite (as you'd need to without testExecutable) then package:test provides root locations that match the test file:

"line": 4,
"column": 3,
"url": "file:///C:/Dev/Test%20Projects/test_setup_diff_file/test/utils.dart",
"root_line": 7,
"root_column": 3,
"root_url": "file:///C:/Dev/Test%20Projects/test_setup_diff_file/test/test_setup_diff_file_test.dart"

I think we can handle this by special-casing these methods and looking at line/col/url for these "tests", although we currently have to do that based on the names. The reason we don't normally use these fields is that if a user calls a test wrapper (like testWithWidgets) we do want the call back from the main suite.

@DanTup
Copy link
Member Author

DanTup commented Aug 9, 2023

We now treat setup/teardown specially and attribute them to their actual locations and not the call in the suite. This means a setup/teardown might be recorded against multiple suites in the tree - right-clicking on the icon will let you select which suite to jump to in the tree (seems VS Code already had good handling of this).

image

@DanTup DanTup closed this as completed in 8f8df52 Aug 9, 2023
@polina-c
Copy link

thank you!!!! it is must have thing for leak_tracker, because analysis is happening in tearDownAll

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in testing Relates to test execution of Dart/Flutter tests for end users is bug
Projects
None yet
Development

No branches or pull requests

2 participants