-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Make StackTraceHelper tests runtime-agnostic #64007
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
base: main
Are you sure you want to change the base?
Conversation
var methodNames = stackFrames.Select(stackFrame => stackFrame.MethodDisplayInfo.ToString()).ToList(); | ||
|
||
// Assert - Runtime-agnostic checks for essential stack trace components | ||
// Instead of exact string matching, verify key components are present |
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.
While you are still verifying that key components are present, it does mean that we are a bit less strict in CoreCLR which could be a potential for regressions. Would it make sense to instead of relaxing some of these matches, to instead first do a Runtime check to see if running under CoreCLR or Mono, and then if CoreCLR we keep the stricter checks but for Mono we just validate key components? This way you are not losing any regression coverage, but you are making the tests pass on more runtimes.
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.
@joperezr Good point! I've updated it with runtime-specific assertions as you suggested
Thank you so much for your contribution @medhatiwari! Left a small comment that would be good if you could address, but otherwise this is looking really good 😃 |
Thanks for reviewing the PR |
Summary
Fixes the remaining 2 failing StackTraceHelper tests from #63927 by making tests runtime-agnostic instead of expecting exact string matches for runtime internals.
Problem
Tests were failing on Mono due to strict string matching against runtime-specific formatting:
"lambda_method34"
but Mono shows"object.lambda_method34"
Solution
Updated tests to verify essential functionality while accepting different valid runtime behaviors:
Assert.StartsWith("lambda_method")
toAssert.Contains("lambda_method")
cc: @giritrivedi