Skip to content

Add logging for spinquic tests. - #5643

Merged
Jack He (ProjectsByJackHe) merged 2 commits into
mainfrom
jackhe/add-stress-logs
Dec 9, 2025
Merged

Add logging for spinquic tests.#5643
Jack He (ProjectsByJackHe) merged 2 commits into
mainfrom
jackhe/add-stress-logs

Conversation

@ProjectsByJackHe

Copy link
Copy Markdown
Contributor

Description

The Spin powershell script had support for gathering MsQuic logs but the CI is never using it.

Let's add an option to configure logging in spin tests. For workflow dispatch, the user can control the log profile. For pull request and merge events, Basic.Light is used.

Testing

CI

Documentation

N/A

type: choice
options:
- "None"
- SpinQuic.Light

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Wouldn't this one make more sense as a default?
Or if it isn't enough (look like it might contain very little), maybe we should consider adding what makes sense to it - no point in having a SpinQuic profile that is not enough to diagnose issues.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, we'll see what these look like in the next few iterations.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sounds good. If it doesn't appear useful, I would not hesitate to delete the profile from the wprpr file and replace it by less specific one.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Seems good to me in concept.

Most of the time though, spin test matter only when they trigger a crash. Shouldn't we focus on dump collection?

@ProjectsByJackHe

Jack He (ProjectsByJackHe) commented Dec 9, 2025

Copy link
Copy Markdown
Contributor Author

Seems good to me in concept.

Most of the time though, spin test matter only when they trigger a crash. Shouldn't we focus on dump collection?

If the script does what it's supposed to do, having log-profile set to non-NULL should automatically coalesce all additional logs created into the artifacts folder. Today, the artifacts folder does not contain any ETL.

@codecov

codecov Bot commented Dec 9, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.75%. Comparing base (4e84609) to head (f94e507).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5643      +/-   ##
==========================================
- Coverage   86.34%   85.75%   -0.59%     
==========================================
  Files          60       60              
  Lines       18663    18663              
==========================================
- Hits        16114    16004     -110     
- Misses       2549     2659     +110     

☔ 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.

@ProjectsByJackHe

Copy link
Copy Markdown
Contributor Author

Seems good to me in concept.

Most of the time though, spin test matter only when they trigger a crash. Shouldn't we focus on dump collection?

Also, if what you mean by "dump collection" is a crash dump, the spinquic tool does not produce any crash dumps in user mode.

@ProjectsByJackHe
Jack He (ProjectsByJackHe) merged commit 0b1071e into main Dec 9, 2025
545 of 549 checks passed
@ProjectsByJackHe
Jack He (ProjectsByJackHe) deleted the jackhe/add-stress-logs branch December 9, 2025 22:26
Masahiro Kozuka (masa-koz) pushed a commit to masa-koz/msquic that referenced this pull request Dec 21, 2025
## Description

The Spin powershell script had support for gathering MsQuic logs but the
CI is never using it.

Let's add an option to configure logging in spin tests. For workflow
dispatch, the user can control the log profile. For pull request and
merge events, Basic.Light is used.

## Testing

CI

## Documentation

N/A
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