-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Maven task does not find the task-report.txt is some cases #2559
Conversation
@@ -2,6 +2,7 @@ | |||
|
|||
import Q = require('q'); | |||
import path = require('path'); | |||
import glob = require('glob'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you checked LCA approval for this module?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Glob is used extensively throught the tasks.
@@ -80,6 +80,8 @@ | |||
"loc.messages.sqAnalysis_QualityGatePassed": "The SonarQube quality gate associated with this build has passed (status %s)", | |||
"loc.messages.sqAnalysis_UnknownComparatorString": "The SonarQube build summary encountered a problem: unknown comparator '%s'", | |||
"loc.messages.sqAnalysis_NoUnitsFound": "The list of SonarQube measurement units could not be retrieved from the server.", | |||
"loc.messages.sqAnalysis_NoReportTask": "Could not find report-task.txt. Possible cause: the SonarQube analysis did not complete successfully.", | |||
"loc.messages.sqAnalysis_MultipleReportTasks": "Multiple report-task.txt files found. Choosing the first one. The build summary and the build breaker may not be accurate. Possible cause: multiple SonarQube analysis during the same build, which is not supported.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not aware that this limitation is noted anywhere in the docs, do they say anything about this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is self documented through the warming message. The docs are general purpose and do not list obscure scenarios.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a user wished to try this, the first time they are going to know that it isn't supported is when their build fails. This doesn't sound like a good experience to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It isn't going to be a good experience for them anyway - their test results will be all mixed up and their code coverage data confused. The point is to give the user enough information to log an issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's why I'm asking if the online docs say anything, so they don't have to get to that point before they find out it's not supported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty sure I already answered your question.
@@ -50,7 +50,8 @@ export class SonarQubeRunSettings { | |||
* @returns {SonarQubeRunSettings} A new SonarQubeRunSettings with appropriate fields filled | |||
*/ | |||
public static createRunSettingsFromFile(filePath:string): SonarQubeRunSettings { | |||
if (!tl.exist(filePath)) { | |||
|
|||
if (!fs.existsSync(filePath)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is deprecated and should not be used in production code. See the following:
https://nodejs.org/api/fs.html#fs_fs_existssync_path
http://stackoverflow.com/questions/28532820/fs-exists-fs-existssync-why-are-they-deprecated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at this.
export function processSonarQubeIntegration(sqBuildFolder:string):Q.Promise<void> { | ||
var sqRunSettings:SonarQubeRunSettings = getSonarQubeRunSettings(sqBuildFolder); | ||
var sqMetrics:SonarQubeMetrics = getSonarQubeMetrics(sqRunSettings); | ||
export function processSonarQubeIntegration(sqTaskReportGlob: string): Q.Promise<void> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main entry point into the post processing we do around SQ changes to accept a glob path. Note that is code is common to both Gradle and Maven and it is build agnostic.
@@ -2,6 +2,7 @@ | |||
|
|||
import Q = require('q'); | |||
import path = require('path'); | |||
import glob = require('glob'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Glob is used extensively throught the tasks.
@@ -80,6 +80,8 @@ | |||
"loc.messages.sqAnalysis_QualityGatePassed": "The SonarQube quality gate associated with this build has passed (status %s)", | |||
"loc.messages.sqAnalysis_UnknownComparatorString": "The SonarQube build summary encountered a problem: unknown comparator '%s'", | |||
"loc.messages.sqAnalysis_NoUnitsFound": "The list of SonarQube measurement units could not be retrieved from the server.", | |||
"loc.messages.sqAnalysis_NoReportTask": "Could not find report-task.txt. Possible cause: the SonarQube analysis did not complete successfully.", | |||
"loc.messages.sqAnalysis_MultipleReportTasks": "Multiple report-task.txt files found. Choosing the first one. The build summary and the build breaker may not be accurate. Possible cause: multiple SonarQube analysis during the same build, which is not supported.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is self documented through the warming message. The docs are general purpose and do not list obscure scenarios.
if (!tl.exist(filePath)) { | ||
public static createRunSettingsFromFile(filePath: string): SonarQubeRunSettings { | ||
|
||
let stats = fs.statSync(filePath); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused variable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* Search for task-report.txt instead of picking it up from a fixed location * Update Gradle to use the same aproach * fs exists sync * Remove unused var
Hi Team, Looks like "Maven task does not find the task-report.txt is some cases" issue has been fixed. Just wanted to know how to get the updated version in our TFS instance, we are using in on premise Thanks |
#2485