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

[Azure] Add A Renderer For Azure DevOps Pipeline #338

Merged
merged 7 commits into from
Dec 10, 2024

Conversation

weisunOW
Copy link
Contributor

@weisunOW weisunOW commented Nov 25, 2024

Description

This PR adds a renderer to annotate build logs for Azure DevOps pipelines. The new AzureDevOpsPipelinesRenderer shares most formatting logic with GitHubActionsRenderer.

E.g.

xcodebuild [Flags] 2>&1 | xcbeautify --renderer azure-devops-pipelines

Or

swift test 2>&1 | xcbeautify --renderer azure-devops-pipelines

References

Azure DevOps - Task Commands

@weisunOW weisunOW requested a review from cpisciotta as a code owner November 25, 2024 12:00
Comment on lines 237 to 236
func formatSwiftTestingIssue(group: SwiftTestingIssueCaptureGroup) -> String {
let message = "Recorded an issue" + (group.issueDetails.map { " (\($0))" } ?? "")
return outputGitHubActionsLog(
annotationType: .notice,
return makeOutputLog(
annotation: .error,
message: message
)
}

func formatSwiftTestingIssueArguments(group: SwiftTestingIssueArgumentCaptureGroup) -> String {
let message = "Recorded an issue" + (group.numberOfArguments.map { " (\($0)) argument(s)" } ?? "")
return outputGitHubActionsLog(
annotationType: .notice,
return makeOutputLog(
annotation: .error,
message: message
)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed the annotation for these two methods to .error as it reflects what's shown in the Xcode.

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks! If it's not too much trouble, can you split this out into its own PR? Happy to do that if you'd like – just let me know if you'd like me to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I will raise a separate PR soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in #340, I will rebase this branch once #340 is merged.

@weisunOW
Copy link
Contributor Author

I will revert the test back to XCTest sometime tonight (UTC+11).

Copy link

codecov bot commented Nov 27, 2024

Codecov Report

Attention: Patch coverage is 97.85714% with 3 lines in your changes missing coverage. Please review.

Project coverage is 89.48%. Comparing base (e81671f) to head (659a8d4).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...Renderers/Microsoft/MicrosoftOutputRendering.swift 95.34% 2 Missing ⚠️
...urces/XcbeautifyLib/Renderers/FileComponents.swift 94.44% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #338      +/-   ##
==========================================
+ Coverage   88.86%   89.48%   +0.62%     
==========================================
  Files          14       17       +3     
  Lines        2291     2312      +21     
==========================================
+ Hits         2036     2069      +33     
+ Misses        255      243      -12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Owner

@cpisciotta cpisciotta left a comment

Choose a reason for hiding this comment

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

Hey @weisunOW! Thanks so much for your patience and contribution! This looks great to me, and I'm glad folks will be able to use this on Azure CI soon.

Quick question: Have you tested these changes in Azure's CI to see that it renders as you expect? If not, can you do that (and possibly link for me to check out)?

Thanks again... Looks good to approve once we address my one comment and question.

Comment on lines 237 to 236
func formatSwiftTestingIssue(group: SwiftTestingIssueCaptureGroup) -> String {
let message = "Recorded an issue" + (group.issueDetails.map { " (\($0))" } ?? "")
return outputGitHubActionsLog(
annotationType: .notice,
return makeOutputLog(
annotation: .error,
message: message
)
}

func formatSwiftTestingIssueArguments(group: SwiftTestingIssueArgumentCaptureGroup) -> String {
let message = "Recorded an issue" + (group.numberOfArguments.map { " (\($0)) argument(s)" } ?? "")
return outputGitHubActionsLog(
annotationType: .notice,
return makeOutputLog(
annotation: .error,
message: message
)
}
Copy link
Owner

Choose a reason for hiding this comment

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

Thanks! If it's not too much trouble, can you split this out into its own PR? Happy to do that if you'd like – just let me know if you'd like me to.

@weisunOW
Copy link
Contributor Author

weisunOW commented Dec 9, 2024

Hi @cpisciotta, happy to help.

Quick question: Have you tested these changes in Azure's CI to see that it renders as you expect? If not, can you do that (and possibly link for me to check out)?

I've verified the branch on one of the pipelines of my enterprise project. The update renders fine on Azure Pipelines for the included logs under Tests/ParsingTests. However, when testing on my enterprise project, it failed to annotate a few PhaseScriptExecution logs and some consecutive warning logs for both GitHubActionsRenderer and the new AzureDevopsPipelineRenderer. I couldn't figure out why because the Regex from the CaptureGroup matches fine on these missed logs.

Microsoft temporarily disabled free grants for new organisations on Azure DevOps until the end of this year, so I can't provide a public pipeline for you to check. Unfortunately, I also can't share the logs from the enterprise pipeline due to policy.

cpisciotta pushed a commit that referenced this pull request Dec 10, 2024
## Description

This PR updates the annotation type for Swift Testing issue recording
(`.notice` -> `.error`), aligning with both Xcode and `swift test`
output.

## References

See discussion in
#338 (comment)

Co-authored-by: Wei Sun <[email protected]>
Copy link
Owner

@cpisciotta cpisciotta left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks again for contributing this change 🚀

@cpisciotta cpisciotta merged commit dcda768 into cpisciotta:main Dec 10, 2024
13 checks passed
@cpisciotta
Copy link
Owner

@weisunOW It's late my time, so I will follow up on releasing a new version – likely tomorrow night.

@weisunOW
Copy link
Contributor Author

Thanks @cpisciotta, looking forward to the new release.

@weisunOW weisunOW deleted the ws-azure-renderer branch December 10, 2024 05:31
@cpisciotta
Copy link
Owner

@weisunOW This is now available via 2.16.0. Homebrew should pick it up shortly. Thanks again!

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