Skip to content

Conversation

@mdaigle
Copy link
Contributor

@mdaigle mdaigle commented Dec 4, 2025

Converted XEventScope to a class because ref struct types are only available starting in c# v13 and this branch uses v9.

Includes various fixes to enable compilation on linux.

Copilot AI review requested due to automatic review settings December 4, 2025 18:17
@mdaigle mdaigle changed the base branch from main to release/6.0 December 4, 2025 18:18
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR backports fixes from the main branch (#3775, #3819) to the 6.0 branch, addressing XEvent test failures and adding CodeQL workflow support. The key change involves converting XEventScope from a ref struct to a class to maintain C# v9 compatibility (ref struct interfaces require C# v13).

  • Adds comprehensive XEventScope utility class to manage XEvent sessions with automatic cleanup
  • Refactors test infrastructure to use instance methods with ITestOutputHelper for proper test name extraction
  • Implements a type-safe ServerProperty enum for querying SQL Server properties
  • Adds CodeQL security scanning workflow for the repository

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
XEventsTracingTest.cs New test validating that client-side activity IDs are correctly passed to server-side XEvent sessions
DataStreamTest.cs Converts test class from static to instance-based; refactors XEvent streaming test to use XEventScope utility; contains unresolved merge conflict
DataTestUtility.cs Adds XEventScope helper class, CurrentTestName method, and ServerProperty enum; refactors GetSqlServerProperty for type safety
codeql.yml New GitHub Actions workflow for CodeQL security scanning; configured for 'main' branch instead of '6.0' branch

Copilot AI review requested due to automatic review settings December 4, 2025 18:56
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 16 comments.

Copilot AI review requested due to automatic review settings December 4, 2025 19:41
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.

mdaigle and others added 3 commits December 4, 2025 16:26
* Upgrade / align packages in test projects /

* Reference System.ServiceProcess on netfx

* Always include System.Transactions as well

* Remove references to xunit.runner.console and xunit.runner.utility to enable discovery of netfx tests again. See dotnet/runtime#94183

* Revert nuget.org addition

* Upgrade Versions
Remove VersionsNet8OrLater, not needed anymore

* Force build

* Remove dummy

* Add back console runners

* Remove System.ServiceProcess, should not be needed

* I think we do actually still need the net9 versions

---------

Co-authored-by: Michel Zehnder <MichelZ@users.noreply.github.com>
Copilot AI review requested due to automatic review settings December 5, 2025 00:43
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 10 comments.

Copilot AI review requested due to automatic review settings December 5, 2025 19:33
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.

@mdaigle mdaigle changed the title [6.0] Fix xevent and codeql (#3775)(#3819) [6.0] Fix xevent and codeql (#3775)(#3819), add global.json Dec 5, 2025
@mdaigle mdaigle marked this pull request as ready for review December 5, 2025 20:02
@mdaigle mdaigle requested a review from a team as a code owner December 5, 2025 20:02
paulmedynski
paulmedynski previously approved these changes Dec 5, 2025
cheenamalhotra
cheenamalhotra previously approved these changes Dec 5, 2025
@mdaigle mdaigle dismissed stale reviews from cheenamalhotra and paulmedynski via b666719 December 5, 2025 22:01
@paulmedynski paulmedynski self-assigned this Dec 8, 2025
@codecov
Copy link

codecov bot commented Dec 8, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.58%. Comparing base (8b320de) to head (b666719).
⚠️ Report is 1 commits behind head on release/6.0.

Additional details and impacted files
@@               Coverage Diff                @@
##           release/6.0    #3827       +/-   ##
================================================
+ Coverage        75.82%   92.58%   +16.75%     
================================================
  Files              244        6      -238     
  Lines            40221      310    -39911     
================================================
- Hits             30497      287    -30210     
+ Misses            9724       23     -9701     
Flag Coverage Δ
addons 92.58% <ø> (ø)
netcore ?

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mdaigle mdaigle merged commit fd34d5e into release/6.0 Dec 9, 2025
241 checks passed
@mdaigle mdaigle deleted the dev/mdaigle/port-xevent-codeql branch December 9, 2025 01:39
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.

5 participants