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

#71252 disable Atan2PiTest on Android x86 #71382

Merged
merged 3 commits into from
Jun 30, 2022

Conversation

mkhamoyan
Copy link
Contributor

Disable Atan2PiTest on Android x86 #71252

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@ghost ghost assigned mkhamoyan Jun 28, 2022
@mkhamoyan
Copy link
Contributor Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mkhamoyan mkhamoyan requested a review from steveisok June 28, 2022 12:46
@@ -1486,7 +1486,7 @@ public static void Atan2PiTest(double y, double x, double expectedResult, double
AssertExtensions.Equal(+expectedResult, double.Atan2Pi(+y, +x), allowedVariance);
}

[Theory]
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotAndroid), nameof(PlatformDetection.IsNotX86Process))] // disabled on Android x86, see https://github.com/dotnet/runtime/issues/71252
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this disable the test on all x86, not just for Android?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my understanding it works like PlatformDetection.IsNotAndroid && PlatformDetection.IsNotX86Process

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, so if it is x86, it fails the right clause, and that whole expression is false.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see , I will add new property in PlatformDetection then checking IsNotAndroid || (IsAndroid && IsNotX86Process)

Copy link
Member

@stephentoub stephentoub Jun 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then checking IsNotAndroid || (IsAndroid && IsNotX86Process)

You just want !(IsAndroid && Isx86)

@ghost
Copy link

ghost commented Jun 28, 2022

Tagging subscribers to 'arch-android': @steveisok, @akoeplinger
See info in area-owners.md if you want to be subscribed.

Issue Details

Disable Atan2PiTest on Android x86 #71252

Author: mkhamoyan
Assignees: mkhamoyan
Labels:

os-android, area-Codegen-meta-mono

Milestone: -

@mkhamoyan
Copy link
Contributor Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mkhamoyan
Copy link
Contributor Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mkhamoyan mkhamoyan merged commit d04a589 into dotnet:main Jun 30, 2022
@akoeplinger
Copy link
Member

Thanks. There's a good chance we'll be able to reenable those tests once #65723 lands.

@ghost ghost locked as resolved and limited conversation to collaborators Aug 10, 2022
@mkhamoyan mkhamoyan deleted the 71252_disable_atanpitest_android branch August 19, 2022 12:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants