vscode-coverage-gutters icon indicating copy to clipboard operation
vscode-coverage-gutters copied to clipboard

Issue with relative paths outside the workspace

Open jonculver opened this issue 4 years ago • 9 comments

Describe the bug For reasons outside of my control my XML looks like this:

<source>/remote/coverage/results</source>

<class filename="a/b/c/my_code.c" >

The filename is thus a relative path but the source points to a location outside the workspace. The source file exists in both places since the coverage tool archived it alongside the results, so either of the following paths are valid:

  1. /remote/coverage/results/a/b/c/my_code.c
  2. {$workspace}/a/b/c/my_code.c

[EDIT after doing more investigation]

The problem is that section.file is constructed from the XML and results in path 1 but this then fails to match the open files in the editor which use path 2.

I can work around this using the following config, which turns the absolute path constructed from the XML back into a relative path. However this relies on knowing that path (or at least the last part of it) and I don't necessarily have this information.

"coverage-gutters.remotePathResolve": ["/remote/coverage/results/", "./"],

Is it possible to add a config knob that has the effect of ignoring the source path in the XML and using workspace root instead?

Desktop (please complete the following information):

  • OS: linux
  • Extension Version [e.g. 2.5.1]
  • VSCode Version [e.g. 1.47.0]

jonculver avatar Jul 15 '20 16:07 jonculver

Hey @jonculver thanks for the github issue!

Hmm seems like an interesting issue, can you generate the coverage in any other formats or are you stuck with the above format? (the extension currently supports the following for xml: CLOVER, JACOCO, COBERTURA)

Part of the issue is that currently you need to have the coverage in the xml be relative (can you generate it with that mode instead?)

    /**
     * Checks for a matching section file against the a given fileName
     * @param section data section to check against filename
     * @param editorFileRelative normalized relative path (against workspace folder) of editor filename, starts with ###
     * @param workspaceFolderName workspace folder name
     */
    private checkSection(section: Section, editorFileRelative: string, workspaceFolderName: string): boolean {
        // Check if we need to swap any fragments of the file path with a remote fragment
        // IE: /var/www/ -> /home/me/
        const sectionFileName = this.resolveFileName(section.file);
        if (!isPathAbsolute(sectionFileName)) {
            return this.checkSectionRelative(sectionFileName, editorFileRelative);
        } else {
            return this.checkSectionAbsolute(sectionFileName, editorFileRelative, workspaceFolderName);
        }
    }

There could be a config knob like you mentioned but there would be a bit of work to do in this section to allow for more mutation along the lines of the existing remotePathResolve feature.

ryanluker avatar Jul 17 '20 01:07 ryanluker

Thanks for the response. The XML in this case is in cobertura format. I don't have control over the generation tool, which is trying to be helpful by archiving the source files and then reporting coverage for the files in the archived location. (It does this because it is usually run as a cloud service and the workspace wouldn't persist after the testing was complete. However in this case I'm running it locally and want to overlay the coverage output on my workspace source.)

jonculver avatar Jul 29 '20 09:07 jonculver

Just to add some more info in case someone else finds this issue, I'm generating the code coverage (using pytest-cov) inside a Docker container, so the path is completely different from the local workspace.

The workaround solved the issue, but even better, I found that if I set:

[run]
relative_files = True

in .coveragerc then I get <source>.</source> in the XML output which fixes it.

For reference, see: https://coverage.readthedocs.io/en/latest/config.html#config-run-relative-files

This all being said, some functionality to detect if the absolute path doesn't exist and falling back to relative would be super useful as currently I'll need to update .coveragerc in all our microservices.

daaain avatar Jun 18 '21 13:06 daaain

OK, another issue, in different repos we have different paths inside Docker (different for a library or a microservice for example), but the array indices are hardcoded so it's only possible to have one mapping.

I guess another possible improvement would be allowing multiple mappings where the last value is localFragment and the rest are possible remoteFragments?

daaain avatar Jun 18 '21 17:06 daaain

@daaain Thanks for the comments! RE Comment 1 around falling back to relative: It should currently try to use absolute and fallback to relative if it didn't resolve properly 🤔 . This check is just looking at the file path and not actively trying to load the file which might be the issue? https://github.com/ryanluker/vscode-coverage-gutters/blob/master/src/coverage-system/sectionfinder.ts#L123-L151

RE Comment 2 around multiple fragments: Correct we could allow for more then the 2 file paths with the expectation you outlined, we probably should make that a new ticket though as I don't think it is related to the original issue of having the workspace folder be used instead of the source. https://github.com/ryanluker/vscode-coverage-gutters/blob/master/src/coverage-system/sectionfinder.ts#L76-L96

ryanluker avatar Jun 19 '21 17:06 ryanluker

Hmm, that fallback to relative sounds perfect, but for me without remotePathResolve nothing gets rendered. I can see that the extension is picking up coverage file changes in the output console and I get RENDERING READY but nothing is rendered. Is there a way to turn on more verbose output or debug somehow to see what file path is being resolved?

This is how the top of coverage.xml currently looks like:

<?xml version="1.0" ?>
<coverage branch-rate="0.7221" branches-covered="1037" branches-valid="1436" complexity="0" line-rate="0.8431" lines-covered="7113" lines-valid="8437" timestamp="1623412108609" version="5.4">
	<!-- Generated by coverage.py: https://coverage.readthedocs.io -->
	<!-- Based on https://raw.githubusercontent.com/cobertura/web/master/htdocs/xml/coverage-04.dtd -->
	<sources>
		<source>/app</source>
	</sources>
	<packages>
		<package branch-rate="0" complexity="0" line-rate="0.1272" name=".">
			<classes>
				<class branch-rate="1" complexity="0" filename="app.py" line-rate="0" name="app.py">
					<methods/>
					<lines>
						<line hits="0" number="1"/>
						<line hits="0" number="3"/>
						<line hits="0" number="4"/>
						<line hits="0" number="6"/>
					</lines>
				</class>
...

daaain avatar Jun 21 '21 11:06 daaain

@daaain Thanks for the further info, there was an issue recently with cobertura I believe that might be related but not 100% sure that is the cause of your troubles🤔. https://github.com/ryanluker/vscode-coverage-gutters/issues/304#issuecomment-820699984

RE missing renderings:

Can you try to get cobertura to have the source be the absolute path? See the python example project for something to use as guidance. https://github.com/ryanluker/vscode-coverage-gutters/tree/master/example/python

<?xml version="1.0" ?>
<coverage branch-rate="0" branches-covered="0" branches-valid="0" complexity="0" line-rate="0.75" lines-covered="9" lines-valid="12" timestamp="1533493296260" version="4.4.2">
	<!-- Generated by coverage.py: https://coverage.readthedocs.io -->
	<!-- Based on https://raw.githubusercontent.com/cobertura/web/master/htdocs/xml/coverage-04.dtd -->
	<sources>
		<source>C:\dev\vscode-coverage-gutters\example\python\python\foobar</source>
	</sources>
	<packages>

I know you mentioned preferring relative paths but the extension doesn't work as well with those given some constraints in loading files from disk.

RE extra logging:

Whatever shows up in the coverage-gutters log area in the output tab is all the logging we currently have sadly. I would have to add extra or generate you a custom vsix if we wanted more 🤔 . image

Conclusion

There might be an issue between the resolving of the file paths (when remotePathResolve is on) and the relative file path fallback feature if I had to guess 🤔 . You could try investigating the remote path resolve that is setup in the example project for remote-node https://github.com/ryanluker/vscode-coverage-gutters/blob/master/example/example.code-workspace#L10 and see if that helps any.

ryanluker avatar Jun 28 '21 00:06 ryanluker

Hmm, don't think the issues are related, if I use remotePathResolve or set relative_files = True everything seems to work perfectly so the Cobertura file itself seems fine. I just have the same issue as the OP that the absolute path is set to what it is in the container and there's no easy way to make it be the workstation path.

I was just hoping that the fallback to relative paths could be improved so that we don't have to use one of the workarounds. If you think adding this as a test case and trying to fix it would be a suitable first contribution I can maybe give it a go? Oh, and maybe adding a log line what was the final path the plugin decided on?

daaain avatar Jun 28 '21 15:06 daaain

@daaain I would agree on the issues not being related, just something that crossed my mind. On trying out a fix, sure that sounds great! If you wanted something smaller first, definitely the log message would be a big win and we can easily get that out in the next minor / patch release.

ryanluker avatar Jul 01 '21 00:07 ryanluker