Skip to content
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

Fix unconditional success in CLR tests #100905

Merged
merged 1 commit into from
Apr 11, 2024

Conversation

tomeksowi
Copy link
Contributor

Test introduced in #100289 failed the build of CLR tests for e.g. RISC-V:

/runtime/src/tests/Loader/classloader/explicitlayout/Regressions/100220/Runtime_100220.cs(15,23): error XUW1002: Tests should not unconditionally return 100. Convert to a void return.

Part of #84834, cc @dotnet/samsung

Test introduced in dotnet#100289 failed the build of CLR tests for e.g. RISC-V:
```
/runtime/src/tests/Loader/classloader/explicitlayout/Regressions/100220/Runtime_100220.cs(15,23): error XUW1002: Tests should not unconditionally return 100. Convert to a void return.
```
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Apr 11, 2024
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Apr 11, 2024
@jakobbotsch
Copy link
Member

This isn't the first time an issue like this makes it through CI... #94146 and #94531 are similar instances. We should figure out why this doesn't fail in CI. cc @markples

@jakobbotsch jakobbotsch merged commit a8158c1 into dotnet:main Apr 11, 2024
72 checks passed
@markples
Copy link
Member

Question for @tomeksowi and any others who have hit these - what are you doing to expose this? Are you possibly running on a branch with a very old dotnet/main?

I believe that #91235 disabled the analyzers due to the addition of '$(_CLRTEstNeedsToRun)' == 'true' in src/test/Directory.Build.targets. @ivdiazsa any chance that you remember why you needed to add this? I imagine that the proper fix is to separate the analyzers into a pure analyzer phase as @jkoritzinsky as suggested in the past. I don't know how to do this and think it would probably take me a while to figure out - if there was a pattern that I could copy then I could probably do it easily.

@gbalykov
Copy link
Member

@markples for both cases when I fixed similar ones our CI built latest main branch from scratch in clean cloned repo, so all nugets and sdk were downloaded during build. Maybe the way we build has some effect, but I'm not exactly sure right now for which configuration we've observed this. It probably was smth like #97791 (comment).

@markples
Copy link
Member

Thank you @gbalykov - the BuildAsStandalone is the difference. To be clear, this isn't incorrect on your part, but it is different from the CI jobs and most local builds. It's useful to know this because it is consistent with my theory above.

matouskozak pushed a commit to matouskozak/runtime that referenced this pull request Apr 30, 2024
Test introduced in dotnet#100289 failed the build of CLR tests for e.g. RISC-V:
```
/runtime/src/tests/Loader/classloader/explicitlayout/Regressions/100220/Runtime_100220.cs(15,23): error XUW1002: Tests should not unconditionally return 100. Convert to a void return.
```
@github-actions github-actions bot locked and limited conversation to collaborators May 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
community-contribution Indicates that the PR has been added by a community member needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants