Skip to content
This repository has been archived by the owner on Nov 20, 2019. It is now read-only.

Feature/sonar generic data format #58

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

djgilcrease
Copy link

This adds the ability to output in the Sonar Qube Generic Test Data format
https://docs.sonarqube.org/display/SONAR/Generic+Test+Data

I created a new template for the sonar output, and added a template function that allows you to get the test file a test exists in as required by the Generic Test Data format

@djgilcrease
Copy link
Author

This creates output that looks like

<?xml version="1.0" encoding="UTF-8"?>
<testExecutions version="1">
  <file path="github.com/group/project/cmd/server/repository/repository_test.go">
    <testCase name="TestGetPolicy_ValidPolicyName" duration="0">
      <failure message="error">
        <![CDATA[	repository_test.go:67: Expected to find policy schema for tcp-proxy
	repository_test.go:71: Expected to find policy schema for tcp-proxy to match test data
	repository_test.go:77: Expected to find policy schema for tcp-proxy
	repository_test.go:81: Expected to find policy schema for tcp-proxy to match test data]]>
      </failure>
    </testCase>
  </file>
  <file path="github.com/group/project/cmd/server/repository/repository_test.go">
    <testCase name="TestGetPolicy_InvalidPolicyName" duration="0" />
  </file>
  <file path="github.com/group/project/cmd/server/repository/repository_test.go">
    <testCase name="TestGetPolicy_MissingArtifact" duration="0" />
  </file>
</testExecutions>

@tebeka
Copy link
Owner

tebeka commented May 9, 2018

Thanks, I'll have a look soon-ish ...

Copy link
Owner

@tebeka tebeka left a comment

Choose a reason for hiding this comment

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

Thanks for contributing. See my comments in the code. The other main issue is the missing tests. Please write regression tests (see _data directory)

"strings"
)

func fileFileForTest(suite *Suite, test *Test, pkg *build.Package, strip string) string {
Copy link
Owner

Choose a reason for hiding this comment

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

Not sure I like the name fileFile prefix. Hard to understand what this does from the name, maybe a document?

continue
}
contents = string(body)
testFileContents[testFilePath] = contents
Copy link
Owner

Choose a reason for hiding this comment

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

I don't see the test file contents in the XML example, why do we need it?

for _, testFile := range pkg.TestGoFiles {
testFilePath = filepath.Join(pkg.Dir, testFile)
if contents, ok = testFileContents[testFilePath]; !ok {
if body, err = ioutil.ReadFile(testFilePath); err != nil {
Copy link
Owner

Choose a reason for hiding this comment

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

Don't ignore errors, unless there's a good reason


func fileFileForTest(suite *Suite, test *Test, pkg *build.Package, strip string) string {
var (
testFileContents = make(map[string]string)
Copy link
Owner

Choose a reason for hiding this comment

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

We usually don't declare variables in advance, we use := instead

@@ -104,6 +122,14 @@ func escapeForXML(in string) (string, error) {
return w.String(), nil
}

func secondsToMilliseconds(seconds string) string {
t, err := time.ParseDuration(seconds + "s")
Copy link
Owner

Choose a reason for hiding this comment

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

If you already know it's in seconds, why not just do the math directly?

@@ -28,6 +29,7 @@ func init() {
flag.BoolVar(&args.bambooOut, "bamboo", false,
"xml compatible with Atlassian's Bamboo")
flag.BoolVar(&args.xunitnetOut, "xunitnet", false, "xml compatible with xunit.net")
flag.BoolVar(&args.sonarGTDOut, "sonar", false, "xml compatible with Sonar Generic Test Data")
Copy link
Owner

Choose a reason for hiding this comment

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

Need to add a check in main that it's no colliding with other formatting flags.

@djgilcrease
Copy link
Author

Will take a look at these issue this week.

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.

2 participants