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

Fix: Modified how classname is generated to be usable in sonar #36

Merged
merged 1 commit into from
Aug 4, 2015

Conversation

monasuncion
Copy link

These changes will allow assigning of relative path to classnames. Relative path of the test file will be constructed using sonar.tests property and junitReporter "suite" property. I also applied the xml schema change to use this
format (from #26):

< testcase >
< error / >
< /testcase >
< testcase >
< failure />
< /testcase >  

In the case below, test files are located in site-main-php/src/main/webapp/public/js/tests/models directory.

Sonar property example:

sonar.projectName=js
sonar.sources=site-main-php/src/main/webapp/public/js
sonar.projectBaseDir=.
sonar.exclusions=site-main-php/src/main/webapp/public/js/lib/*.js,site-main-php/src/main/webapp/public/js/tests/**/*.php,site-main-php/src/main/webapp/public/js/tests/**/*.js,site-main-php/src/main/webapp/public/js/ec3.3/vendor/**
sonar.javascript.lcov.reportPath=site-main-php/target/coverage/lcov.info
sonar.javascript.jstestdriver.reportsPath=site-main-php/target/surefire-reports/
sonar.tests=site-main-php/src/main/webapp/public/js/tests

Grunt file reporters property example:

reporters: ['junit', 'coverage', 'progress'],
junitReporter: {
    outputDir: $junitResults,
    suite: 'models'
},
coverageReporter: {
    type: 'lcov',
    dir: $coverageOutputDir,
    subdir: '.'
},
preprocessors: {
    'src/main/webapp/public/js/ec3.3/**/*.js': 'coverage',
    'src/main/webapp/public/js/ec3/**/*.js': 'coverage'
},
plugins: [
    'karma-jasmine',
    'karma-phantomjs-launcher',
    'ec-karma-junit-reporter23',
    'karma-coverage'
]

Example junit xml report:

<?xml version="1.0"?>
<testsuite name="PhantomJS 1.9.8 (Linux)" package="models" timestamp="2015-03-10T13:59:23" id="0" hostname="admin" tests="629" errors="0" failures="0" time="11.452">
  <properties>
    <property name="browser.fullName" value="Mozilla/5.0 (Unknown; Linux x86_64) AppleWebKit/534.34 (KHTML, like Gecko) PhantomJS/1.9.8 Safari/534.34"/>
  </properties>
 <testcase name="(C.2) Checks if an empty object is returned when error 404 is encountered" time="0.01" classname="PhantomJS_1_9_8_(Linux).models.ReportCardDialogService_test"/>
 <testcase name="(C.3) Checks if an empty array is returned when error 405 is encountered" time="0.013" classname="PhantomJS_1_9_8_(Linux).models.ReportCardDialogService_test"/>
</testsuite>

@jbarrus
Copy link

jbarrus commented Mar 19, 2015

It looks like this still requires the describe statement to match the filename, e.g.:

something.spec.js

describe('Something Great', function() {
....
});

results in:

 <testcase name="..." time="..." classname="PhantomJS_1_9_8_(Mac_OS_X).Something Great"/>

Sonar looks for a file named "Something Great.js"

I don't really see an easy workaround for this because jasmine doesn't report the filename.

@mdasberg
Copy link

And nested describes like this:

describe('x',function() {
    it('should do something with x', function() {}
    describe('y', function() {
        it('should do something with y', function() {}
    }
}

will generate a report like this:

<testsuite name="SomeTest" errors="0" failures="0" tests="2" time="....">
    <testcase name="should do something with x" time="..." classname="PhantomJS_1_9_8_(Mac_OS_X).x"/>
    <testcase name="should do something with y" time="..." classname="PhantomJS_1_9_8_(Mac_OS_X).x y"/>
</testsuite>

@mpellerin42
Copy link

When this pull request will be merged ?

I need this in my project, but I can't merge it manually because of continuous integration which do a "npm install" on each build...

Thanks

@jbarrus
Copy link

jbarrus commented Jun 22, 2015

Still hoping to get this merged...

@mdasberg
Copy link

I created a grunt plugin which fixes this issue for sonar, by fixing the test repor. It will match each spec to the original files.

Take a look at: https://github.com/mdasberg/grunt-karma-sonar

@dignifiedquire
Copy link
Member

@MoziMos sorry for the late feedback and thanks for the contribution, could you please

  • add some documentation to the readme
  • change the commit message to follow our convention

@dignifiedquire
Copy link
Member

@MoziMos does this supersede #26 ?

@jbarrus
Copy link

jbarrus commented Jun 22, 2015

I believe it does.

@monasuncion monasuncion changed the title Modified how classname is generated to be usable in sonar Fix: Modified how classname is generated to be usable in sonar Jun 23, 2015
@monasuncion
Copy link
Author

yes it does supersede that pull request. I'll add some documentation to the read me doc and update this pull request. Thanks.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@dignifiedquire
Copy link
Member

ping @MoziMos

@googlebot
Copy link

CLAs look good, thanks!

@monasuncion
Copy link
Author

Updated the README.md file. Please review. Thanks!

@dignifiedquire
Copy link
Member

Thanks @MoziMos, could you rebase onto the latest master so I can merge it cleanly squash the commits into one, which follows our convention Then I think it's good to be merged

@monasuncion
Copy link
Author

Hello done with the rebase to the latest master.

@dignifiedquire
Copy link
Member

@MoziMos still need an update to the commit message, and it seems it still won't merge cleanly, are you sure you pulled in the latest master before you rebased?

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

@googlebot
Copy link

CLAs look good, thanks!

Modified how classname is generated to be usable in sonar

Updated Readme.md

Modified README.md

Fix: Change how classname is generated to be usable in sonar

Modified how classname is generated to be usable in sonar

Updated Readme.md

Modified README.md

Removed extra semicolons and spaces

Fix for additional test errors regarding semi colons
dignifiedquire added a commit that referenced this pull request Aug 4, 2015
Fix: Modified how classname is generated to be usable in sonar
@dignifiedquire dignifiedquire merged commit 4d0fed2 into karma-runner:master Aug 4, 2015
@dignifiedquire
Copy link
Member

Thanks :octocat:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants