Add an option to show coverage of failing tests

[myaaaaaaaaa]

VS Code can show code coverage in the editor. Here is an example:

Right now, vscode-go only uses this feature for package-level tests, to show which lines of code aren't being covered by the test suite.

I would like an option to automatically rerun failing tests with coverage enabled upon test failure, which should make them easier to debug by automatically highlighting which lines of code likely caused the failure. An article by Russ Cox on this technique, differential coverage, is available here.

[madelinekalil]

Thank you for your idea, this does seem like it would be a useful feature.

[firelizzard18]

• vscode-go's legacy test system supports coverage; however this is not integrated with the test explorer, it is a completely separate system.
• vscode-go's test explorer does not support coverage.
• Go Companion's test explorer does support coverage.

The previous maintainer (@hyangah) and I planned to merge Go Companion's explorer into vscode-go, replacing vscode-go's explorer and the legacy system. So from that perspective it would make the most sense to me to integrate this into Go Companion's explorer. Unfortunately I have not had as much free time as I would like to work on Go Companion. It's mostly done but I had to do some significant rework to fix a performance issue and there are still a few outstanding bugs (that I know about). I will happily accept pull requests. That being said, I would like a clearer idea of exactly what this would entail before tackling it or accepting a PR. Differential coverage seems like a pretty powerful tool (though not one I've used) but it's not obvious to me what the user experience would be.

I would like an option to automatically rerun failing tests with coverage enabled upon test failure, which should make them easier to debug by automatically highlighting which lines of code likely caused the failure.

Edit: I misunderstood; see here.

I skimmed the article you linked and if I'm understanding it correctly, it requires having coverage before and after the failure. So enabling coverage when a test fails would not work. You'd need to enable a mode where capturing coverage was always enabled, but only displayed when a test changed from passing to failing in order to capture what changed, right? So I think the test system would need to hold on to a coverage file for the last passing run of each test. Unfortunately I don't know that there's sufficient granularity to make that work well; when I execute tests, I get a single coverage file which is a list of code locations and hit counts - there's not enough information to tell which test covered which locations. Unless there's a way to recover that information, I think this feature would require executing every test individually, or at least every test that had the feature enabled.

[myaaaaaaaaa]

Differential coverage seems like a pretty powerful tool (though not one I've used) but it's not obvious to me what the user experience would be.

To be honest, I'm not really sure either. I'm open to feedback. My thinking was to start via the simplest way to integrate it in, and then figure out pain points and better workflows as we go.

How cumbersome would it be to have goTest() call itself on failure if coverage isn't already enabled? Somewhere around here?

vscode-go/extension/src/testUtils.ts

Lines 485 to 496 in 6fc8296

	});
	} catch (err) {
	outputChannel.appendLine(`Error: ${testType} failed.`);
	if (err instanceof Error) {
	outputChannel.appendLine((err as Error).message);
	}
	}
	if (tmpCoverPath) {
	await applyCodeCoverageToAllEditors(tmpCoverPath, testconfig.dir);
	}
	return testResult;
	}

If that's too much, perhaps it's better to just make it a third testing codelens?

 codelens.push(
 	new CodeLens(f.range, {
 		title: 'run test',
 		command: 'go.test.cursor',
 		arguments: [{ functionName }]
 	}),
 	new CodeLens(f.range, {
 		title: 'debug test',
 		command: 'go.debug.cursor',
 		arguments: [{ functionName }]
 	}),
+	new CodeLens(f.range, {
+		title: 'narrow down failures',
+		command: 'go.diffcover.cursor',
+		arguments: [{ functionName }]
+	})
 );

Unless there's a way to recover that information, I think this feature would require executing every test individually, or at least every test that had the feature enabled.

If I'm not mistaken, only three should be required - The initial execution sees which tests are failing. Then, the second one runs all the passing tests with coverage, and the third one runs all the failing tests with coverage. The reports from the second and third ones can then be diffed.

[firelizzard18]

Edit: I misunderstood; see here.

My understanding of https://research.swtch.com/diffcover is that the method is to capture two profiles of the same test, one while it's passing and one while it's failing; IIUC both coverage profiles need to be for the one specific test that started failing.

Then, the second one runs all the passing tests with coverage, and the third one runs all the failing tests with coverage. The reports from the second and third ones can then be diffed.

It's not clear to me what diffing those two coverage profiles (passing tests and failing tests) will do for you. And either way, I don't think that's what RSC is saying in that article.

[myaaaaaaaaa]

From the article (emphases mine):

Let’s collect a passing and failing profile:

+$ go test -coverprofile=c1.prof -skip='TestAddSub$'
 PASS
 coverage: 85.0% of statements
 ok  	math/big	8.373s
+% go test -coverprofile=c2.prof -run='TestAddSub$'
 --- FAIL: TestAddSub (0.00s)

[...]

In the resulting profile, “covered” (green) means it ran in the failing test but not the passing ones, making it something to take a closer look at. Looking at the file list, only natmul.go has a non-zero coverage percentage, meaning it contains lines that are unique to the failing test.

The idea is to be able to easily narrow down a certain class of bugs whose root cause is buried deep inside an edge case, something like:

func getIndex(s string) int {
	n := len(s) - 1
	if rarely {
		n++ //oops, causes index out of range later
	}
	return n
}

[firelizzard18]

Edit: I misunderstood; see here.

Yes, but the critical difference is he is running the same test. He collects a profile for TestAddSub when it is passing, then collects a profile for the exact same test when it is failing. Presumably he made some difference in between collecting the two profiles, such as checking out a different commit. The way I read the section you quoted is:

1. Run TestAddSub (with coverage), it passes.
2. Change something (e.g. git checkout bad-version, or git bisect).
3. Run TestAddSub (with coverage), it fails.
4. Compare the profiles from (1) and (3) to see the difference in coverage.

So to make this work in an automated way, the extension would need to be collecting coverage on every single run, then if a test starts failing, collect coverage again and diff. However I don't think this will work well if the first coverage profile was captured with "run all tests". Because what you really need to know is what lines are/are not covered by that specific test. Let's say you're trying to test getIndex. You have the test you're focused on, which starts failing because something triggers the rare case. But you could also have some other test that triggers the rare case without failing. If you capture coverage on all tests, the other test will mask the fact that the test started failing because of that line. So for this technique to work properly, you need coverage for the specific test that starts failing. The coverage that comes out of go test is just line number and hit count, so there's no way to know which test covered which line. It's possible the actual output of the runtime (which is different from what -coverprofile outputs) includes that info but I'd guess not.

If I'm not mistaken, only three should be required - The initial execution sees which tests are failing. Then, the second one runs all the passing tests with coverage, and the third one runs all the failing tests with coverage. The reports from the second and third ones can then be diffed.

The sequence of events you're proposing is:

1. Run tests, something fails.
2. Run passing tests with coverage.
3. Run failing tests with coverage.

That tells you the difference in coverage between tests that pass and tests that fail. That is not the technique RSC describes. It could be helpful if you add a new test and it fails, but it probably won't be helpful to diagnose the scenario where an existing test starts failing. In my experience, its the "oops I changed some random thing and now I'm getting a failure that seems completely unrelated" scenarios that are where RSC's technique really shines, because those tend to be the hardest failures to diagnose.

[myaaaaaaaaa]

Yes, but the critical difference is he is running the same test. He collects a profile for TestAddSub when it is passing, then collects a profile for the exact same test when it is failing.

No, the first call passes -skip='TestAddSub$' and the second one passes -run='TestAddSub$'

[firelizzard18]

Hm. Well, apparently I'm failing the reading comprehension test today. I guess I got an idea of how coverage could help and didn't properly read what he said. I still think coverage before and after a test started failing would be the most helpful, but since that was my idea that I injected into the conversation (and since I don't know how to make it work while providing a good UX) I'll drop it.

From a UX perspective, I don't love the idea of automatically re-running tests when one failed, especially not if it's on every failure. If it's a fully automatic process, even if it's opt-in, I'd prefer a more specific trigger than just "a test failed". One idea that occurs to me is adding some kind of decoration or other UI indicator that you could click on to trigger the process. "Hey, your test failed, do you want to try differential coverage to diagnose it?" I don't want to generate an alert/notification because that would get obnoxious fast. Unfortunately vscode's testing API is limited and I don't think it has a way to add an indicator icon to the test. Though I'm not at 100% today so maybe I'll think of something when my mind is clearer.

[firelizzard18]

There may be another complication: I'm not sure how Go represents differential coverage under the hood and whether vscode's testing API has a good way of representing it. There's probably a way to make it work, but that is an unknown.

[myaaaaaaaaa]

It seems like coverage is represented simply as a list of character positions to highlight, so it's very flexible regarding actual algorithms. Excerpt from https://stackoverflow.com/questions/31413281/golang-coverprofile-output-format

Format

name.go:line.column,line.column numberOfStatements count

Example

mode: set
github.com/cnuss/api_server/server.go:47.2,48.16 2 0
github.com/cnuss/api_server/server.go:52.2,53.16 2 0
github.com/cnuss/api_server/server.go:57.2,58.16 2 0
github.com/cnuss/api_server/server.go:62.2,63.16 2 0

---

I'm having some second thoughts about differential coverage being one-size-fits-all. Go already provides an option to set binary coverage (did this line run or not?) vs counts (how many times did this line run?), both of which are useful in differential coverage. If anything, the number of coverage-based debugging techniques is probably only going to increase, not consolidate.

Maybe we should try a different angle. We could let the user specify a file path (let's say $COVER_FILE) to watch. Then, every time this file is updated, vscode-go could automatically refresh the editor's coverage display. This would let the user apply arbitrary coverage techniques.