-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Add XUnitLogChecker to log libraries dumps #93906
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
Merged
carlossanlop
merged 63 commits into
dotnet:main
from
carlossanlop:LibrariesXUnitLogChecker
Nov 15, 2023
Merged
Changes from all commits
Commits
Show all changes
63 commits
Select commit
Hold shift + click to select a range
2d1eafc
[DO NOT MERGE | REVERT] - Forced AV in brotli native code. Disable un…
carlossanlop 5242ff3
gen-debug-dump-docs.py: More meaningful messages
carlossanlop 070fd63
XUnitLogChecker.cs: Minor fixes
carlossanlop c178f37
CoreclrTestWrapperLib.cs: Avoid passing null coreRoot string to Path.…
carlossanlop ce72a2a
RunnerTemplate.cmd: Invoke XUnitLogChecker
carlossanlop a57ced6
RunnerTemplate.sh: Invoke XUnitLogChecker
carlossanlop 6d04923
sendtohelix.proj: Set __TestArchitecture and build XUnitLogChecker
carlossanlop 173911f
Fix cmd with a goto because delayed execution prevents setting variab…
carlossanlop 9414c81
Minor sh improvements and verbosity additions
carlossanlop e4c94ac
Move to tests.proj
carlossanlop 02db8bc
Move IsTargetOSSupportedByXUnitLogChecker to root Directory.Build.props.
carlossanlop fb616c5
Simplify project reference in tests.proj
carlossanlop 6e077e6
Remove coredump_filter and ulimit configuration, since helix machines…
carlossanlop 044cb20
Rename IsTargetOSSupportedByXUnitLogChecker to IsXUnitLogCheckerSuppo…
carlossanlop 2732b61
Skip XUnitLogChecker execution in Net48 libraries test runs.
carlossanlop e07e28b
Re-add the accidentally removed HelixCorrelationPayload, but with the…
carlossanlop 52df19c
Build XUnitLogChecker in tests.proj
carlossanlop c04a72a
Add an empty Tests target and a copy of ZipTestArchive to the XUnitLo…
carlossanlop b047c8c
Condition the custom ZipTestArchive inside XUnitLogChecker.csproj
carlossanlop 8189449
Separate framework check from IsXUnitLogCheckerSupported, the value i…
carlossanlop 3bd400f
Change cmd env var to int
carlossanlop 0b5bd2a
typo typo typo
carlossanlop 7123d0f
Separate the directory paths into two, they have different purposes.
carlossanlop 384fdf5
Fix helix path
carlossanlop 7c3d0fe
The zip goes under artifacts, not artifacts/bin
carlossanlop 26db023
Skip in NativeAot tests
carlossanlop 93e33c2
Always set the value of _IsXUnitLogCheckerSupported in the cmd and sh…
carlossanlop d3fe8fa
Add verbose messages for IsXUnitLogCeckerSupported possible values, a…
carlossanlop 01541fc
Try disabling using TestNativeAot instead
carlossanlop cda54c1
Do not attempt to run libs.tests on XUnitLogChecker
carlossanlop ec0774b
Remove temp messages
carlossanlop 2c9ec70
Better name for internal property in sendtohelix.proj
carlossanlop 11fa84a
Temp var that prints literal value of IsXUnitLogCheckerSupported
carlossanlop c955c5e
Move the MSBuildProjectName condition to the RunTests target itself
carlossanlop dfa7985
Remove the test target from XUnitLogChecker.csproj
carlossanlop 299a454
Set OutDir value directly
carlossanlop 942687b
TEMP: Print value of property in send to helix
carlossanlop 4c9a8ff
Put output directory somewhere else so we dont assume XUnitLogChecker…
carlossanlop 6f609e9
Remove the undefined env var check in the cmd.
carlossanlop bf96801
Use BuildTargetFramework to set the env var in sendtohelixhelp.proj
carlossanlop ca3522e
Use NetCoreAppCurrent
carlossanlop 0345424
Fix temp conflict with base
carlossanlop 3ca551a
The Libraries Build leg needs to also publish the artifacts/bin/XUnit…
carlossanlop c417c25
Extra spacing
carlossanlop 5c46f37
Only set envvars inside the condition
carlossanlop c2243dc
The cmd condition does not work unless double quoted
carlossanlop d94e151
TMP: Printed final value
carlossanlop 75a4033
Add fi accidentally removed
carlossanlop d6da591
Move the Linux-only code to its own if, and everything else should ru…
carlossanlop 6568293
Ensure the OSX core folder is also visited.
carlossanlop 85b5e0b
Revert temporary changes
carlossanlop 25f6a70
[DO NOT MERGE | REVERT] Cause a crash and disable unrelated CI legs
carlossanlop f7d7e50
Simplify envvar condition in sendtohelixhelp.proj
carlossanlop b926291
Move build target framework check up to the property group.
carlossanlop 69df4b6
Missed a sh variable $
carlossanlop bb772dd
Remove should set variable from outside the condition, it's unused
carlossanlop 3686646
In OSX, $HELIX_DUMP_FOLDER is passed as `/cores`, so we don't need to…
carlossanlop 87f7055
Missed a ;
carlossanlop 3da51fb
Another missed ;
carlossanlop 0d15bf8
Use the $core_file_name when renaming the dmp
carlossanlop bcd2acf
Remove sleep
carlossanlop 7857f36
Remove $RANDOM
carlossanlop fab3449
Revert the temporary crash and the disabled CI legs.
carlossanlop File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Why do you need to condition on
TestRunNamePrefixSuffix? NativeAOT is already excluded via theTestNativeAotproperty, isn't it?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.
When it was only TestNativeAot, I was still seeing failures last night in some NativeAOT cases. I realized TestRunNamePrefixSuffix was another value that was used when running tests that could help detect aot tests, and now it worked.
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.
Do you need both in that case? Also, runtime flavor should remove a lot of the TargetOS ones.
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.
I noticed some NativeAot runs were not getting ignored unless I added both conditions. They are temporary anyway. I opened an issue so the NativeAOT owners can get it enabled properly.
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.
Having to condition on
'$(TestRunNamePrefixSuffix)' != 'NativeAOT_Release'is extremely suspicious. We only ever run native AOT testing withTestNativeAot. It's not native AOT testing if that property is not true. This will likely produce obscure failure modes if we add new test pipelines or rename existing. It will be expensive to root cause for whoever isn't already aware of this hack. Cc @dotnet/ilc-contrib for awareness.Unless you already talked to some "NativeAOT owners" and scheduled their time to fix this, this temporary workaround is likely to be in the product for a long time, like most temporary workarounds. I'm very much against this condition being checked in as it is, please investigate this. One thing off top of my head is that this might need to check for
RunNativeAotTestAppsinstead, which gates special kinds of library tests that was added in #76109.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.
@MichalStrehovsky Since I had to revert the change from main due to NativeAOT tests being broken in outerloop, I can remove the TestRunNamePrefixSuffix property from the condition in the new PR.
I also opened #94722 to track enabling them in the future.
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.
Thanks! The issue might have some overlap with #94270.
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.
Feel free to mark mine as duplicate of yours if they're tracking the same.