Skip to content
This repository was archived by the owner on Jul 16, 2025. It is now read-only.

Conversation

@Tvli
Copy link
Contributor

@Tvli Tvli commented Sep 10, 2017

PR Contribution

This PR fixes the FunctionFinder's problem so that it finds test names correctly for test suites.

Bug description

We use stretchr's testify suite in our go projects, so the test cases in our projects are like the example below:

type ExampleTestSuite struct {
    suite.Suite
    VariableThatShouldStartAtFive int
}

func (suite *ExampleTestSuite) TestExample() {
    suite.Equal(5, suite.VariableThatShouldStartAtFive)
}

The bug is when FunctionFinder scans test files to find all testName -> testFilePath mappings, it finds the wrong function names. For example, when scans the code above, the FunctionFinder thinks the test name is TestSuite) TestExample, but the correct function name is TestExample.

In later step, the GoJunitParser finds function names in the xml file. Then GoJunitParser uses the function name TestExample to find the filePath in the mapping that FunctionFinder provided. Due to the FunctionFinder found the wrong function name, so the GoJunitParser finds filePath to be null. This null breaks the GoTestReportSaver in line 47:

// Due to value.getFile() returns null,  this line throws error
InputFile file = context.fileSystem().inputFile(predicates.hasAbsolutePath(value.getFile()));

We currently work around this issue by renaming our suites to be ExampleSuite so that it doesn't include the Test keyword. However it would be much better to get this problem fixed so that we can still use generic TestSuite names.

Steps to Reproduce

  1. Use test suite in golang project and name the suite as TestSuite
  2. Generate junit test report.
  3. Run sonar-scanner on this project
    p.s (Only check test metric is enough, since coverage and linter metrics have no problems)

Content of your sonar-project.properties

All default settings

Versions

Server: 6.5
Sonar-Scanner: 3.0.3.778-macosx
Go version: 1.8.3
OS: macOS Sierra 10.12.6
gometalinter: 1.2.1

@thibaultfalque
Copy link
Collaborator

Hi,
Thanks for your PR.

Copy link
Contributor

@danielleberre danielleberre left a comment

Choose a reason for hiding this comment

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

The modification is simple, and some test cases are provided.

@danielleberre danielleberre merged commit e4b6b2d into uartois:master Sep 10, 2017
@Tvli
Copy link
Contributor Author

Tvli commented Sep 10, 2017

Thanks for merging.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants