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

Test and Deprecate Publish HTML Report #67

Merged
merged 20 commits into from
Jan 9, 2024

Conversation

chandrashritii
Copy link
Collaborator

@chandrashritii chandrashritii commented Dec 6, 2023

  • Added code changes to deprecate the Publish HTML Report call using a FF
  • Tested locally; HTML report is not being generated

The changes are behind a Feature Flag.

Local testing

Taking a .xml file as input

With the FF set to OFF -

The HTML report is being generated and uploaded in the artifacts -

image

image

With the FF set to ON :

The HTML report is not being generated and consequently, it is not being published to artifacts

image

image

Shriti Chandra (from Dev Box) added 2 commits December 6, 2023 02:30
Signed-off-by: Shriti Chandra (from Dev Box) <[email protected]>
Signed-off-by: Shriti Chandra (from Dev Box) <[email protected]>
@vinayakmsft
Copy link
Collaborator

Please remove all extra lines and spaces

Shriti Chandra (from Dev Box) added 6 commits December 8, 2023 12:35
Signed-off-by: Shriti Chandra (from Dev Box) <[email protected]>
Signed-off-by: Shriti Chandra (from Dev Box) <[email protected]>
Signed-off-by: Shriti Chandra (from Dev Box) <[email protected]>
Signed-off-by: Shriti Chandra (from Dev Box) <[email protected]>
Signed-off-by: Shriti Chandra (from Dev Box) <[email protected]>
Signed-off-by: Shriti Chandra (from Dev Box) <[email protected]>
@chandrashritii chandrashritii marked this pull request as ready for review December 11, 2023 09:56
Shriti Chandra (from Dev Box) added 12 commits December 14, 2023 13:41
…t testing

Signed-off-by: Shriti Chandra (from Dev Box) <[email protected]>
Signed-off-by: Shriti Chandra (from Dev Box) <[email protected]>
Signed-off-by: Shriti Chandra (from Dev Box) <[email protected]>
Signed-off-by: Shriti Chandra (from Dev Box) <[email protected]>
Signed-off-by: Shriti Chandra (from Dev Box) <[email protected]>
Signed-off-by: Shriti Chandra (from Dev Box) <[email protected]>
Signed-off-by: Shriti Chandra (from Dev Box) <[email protected]>
Signed-off-by: Shriti Chandra (from Dev Box) <[email protected]>
Signed-off-by: Shriti Chandra (from Dev Box) <[email protected]>
Signed-off-by: Shriti Chandra (from Dev Box) <[email protected]>
@@ -31,7 +34,7 @@ public static void Main(string[] args)
ProcessCoverage(config, _cancellationTokenSource.Token);
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Please remove extra line



// Feature Flag for testing and deprecating PublishHTMLReport
if (!IsPublishHTMLReportDeprecationEnabled && config.GenerateHTMLReport)
Copy link
Collaborator

Choose a reason for hiding this comment

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

The default state of FF should be off. When we turn it on , then only it should go inside this for loop. So you can remove the "!" mark for the FF name

@@ -57,7 +57,7 @@ private static void ProcessCoverage(PublisherConfiguration config, CancellationT
var publishTimedout = processor.ParseAndPublishCoverage(config, cancellationToken, new Parser(config, context.TelemetryDataCollector))
.Wait(config.TimeoutInSeconds * 1000, cancellationToken);

if(publishTimedout)
if (publishTimedout)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove extra space

@@ -66,7 +66,7 @@ private static void ProcessCoverage(PublisherConfiguration config, CancellationT
publishSuccess = true;
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

please remove extra line

@chandrashritii chandrashritii merged commit 9520d1f into master Jan 9, 2024
3 checks passed
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.

2 participants