-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
coreclr outerloop test build failure #34220
Comments
@BruceForstall how can I build this configuration locally so I can test out my fix? |
You can build all pri-1 tests with:
Or to build just this one test, something like:
|
Where in the build is the failure happening? I looked at that build and all the generated console logs from helix. Saw some JIT test failures but no obvious build issues. Big logs so could've looked past it. |
Also is the preference to fix the warning (add correct override) or leave test as is and suppress the warning here because the structure of the code is meaningful? |
This is failing in all the Pri1 test builds, e.g., the "CoreCLR Pri1 Test Build Windows_NT x64 checked" job, in the "Build managed test components" section. Don't need to go to the Helix logs. I don't know about this test in particular. Looks like @fadimounir created it last year. Generally, I'd advise fixing the code, but maybe Fadi can advise. |
To fix it ASAP, though, I wouldn't wait for more advice: do what you think is best. |
@BruceForstall you listed two builds here. I only see build errors in one:
Not being pedantic here. Just making sure I fix all the issues. Sorry for beraking this. Didn't realize there was an extra build here or I'd have run the pipeline at PR time. |
Yeah, I was pointing out that before it was a warning, after it was an error. btw, in the error case (after your change), I notice this isn't the only error. I also see (at the bottom of the build log):
|
@jaredpar @BruceForstall the test failing to build is aimed at testing some corner case issue with instantiating stubs on arm, where we had a bug with the calling convention. If the correct expectation is to not let users write C# structs, and override the .Equals without overriding the .GetHashCode, then i can make a change to the test and fix it. Please let me know what the correct expected compiler behavior is. |
Revert the warn as error as it's causing the coreclr outerloop build to break. Investigated fixing the warnings but there are a mix of C# and IL warnings. The latter I'm less sure of the fix. Reverting to unblock and will investigate the warnings in parallel. closes dotnet#34220
@jaredpar I did a quick check in VS. The error seen in the CI is actually a warning in previous versions of Roslyn (CS0659). Is the change from warning to error intentional or a regression? |
@fadimounir no. I flipped the "warn as error" switch in mSBuild and it promoted the warning to error. Have a PR out to undo it. |
Revert the warn as error as it's causing the coreclr outerloop build to break. Investigated fixing the warnings but there are a mix of C# and IL warnings. The latter I'm less sure of the fix. Reverting to unblock and will investigate the warnings in parallel. closes #34220
https://dev.azure.com/dnceng/public/_build/results?buildId=576645&view=logs&j=8524357e-1450-5e10-a984-883a50af28d9&t=ede4d230-8340-53fe-3e6a-8c3ab5208a8a
I believe due to #34155 changing warnings to errors. Previously, this was a warning:
https://dev.azure.com/dnceng/public/_build/results?buildId=575509&view=logs&j=8524357e-1450-5e10-a984-883a50af28d9&t=ede4d230-8340-53fe-3e6a-8c3ab5208a8a
@jaredpar
The text was updated successfully, but these errors were encountered: