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

AllowFailureWithoutError is implemented backwards #5701

Closed
nohwnd opened this issue Sep 2, 2020 · 0 comments · Fixed by #5743
Closed

AllowFailureWithoutError is implemented backwards #5701

nohwnd opened this issue Sep 2, 2020 · 0 comments · Fixed by #5743
Assignees

Comments

@nohwnd
Copy link
Member

nohwnd commented Sep 2, 2020

Issue Description

AllowFailureWithoutError will not prevent the error MSB4181: The "Microsoft.TestPlatform.Build.Tasks.VSTestTask" task returned false but did not log an error. when set to true. It will prevent it when set to false. The default value is true.

This option was added in #5207

Steps to Reproduce

Have a task that does not write error into result, but returns false. See the MSB4181 error written to screen. Change the task and set AllowFailureWithoutError to true. See the error written to screen. Set the value to false. The error is no longer written to screen.

I am seeing this in dotnet test when any test fails and try to suppress the message because we don't write the error the correct MSBuild way. microsoft/vstest#2557

Expected Behavior

To not write the MSB4181 error on screen when AllowFailureWithoutError is set to true, not false. And for AllowFailureWithoutError to default to false, not true.

Actual Behavior

MSB4181 error is written to screen when AllowFailureWithoutError is set to true, not false. AllowFailureWithoutError defaults to true.

Analysis

if (taskReturned && !taskResult && !taskLoggingContext.HasLoggedErrors && (be is TaskHost th ? th.BuildRequestsSucceeded : false) && (be is IBuildEngine7 be7 ? be7.AllowFailureWithoutError : true))

This condition will write the MSB4181 when the condition is true.
(be is IBuildEngine7 be7 ? be7.AllowFailureWithoutError : true)) should negate AllowFailureWithoutError like this:
(be is IBuildEngine7 be7 ? !be7.AllowFailureWithoutError : true)).

I don't know where you set the default value.

Versions & Configurations

Microsoft (R) Build Engine version 16.8.0-preview-20429-01+d58e2b786 for .NET

Attach a binlog

@nohwnd nohwnd added bug needs-triage Have yet to determine what bucket this goes in. labels Sep 2, 2020
@Forgind Forgind self-assigned this Sep 16, 2020
@Forgind Forgind removed the needs-triage Have yet to determine what bucket this goes in. label Sep 16, 2020
Forgind added a commit to Forgind/msbuild that referenced this issue Sep 17, 2020
Fixes dotnet#5701.

Changes its meaning without altering the default behavior. Also fixed tests and added one to verify that the default is as expected.
Forgind added a commit that referenced this issue Oct 2, 2020
Fixes #5701.

Changes its meaning without altering the default behavior. Also fixed tests and added one to verify that the default is as expected.
sujitnayak pushed a commit to NikolaMilosavljevic/msbuild that referenced this issue Oct 12, 2020
Fixes dotnet#5701.

Changes its meaning without altering the default behavior. Also fixed tests and added one to verify that the default is as expected.
@AR-May AR-May added the triaged label Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants