Skip to content

Conversation

@MaximLipnin
Copy link
Contributor

@MaximLipnin MaximLipnin commented Dec 14, 2021

Just blindly disabling several crypto tests near which crashes have been observed for a while (based on a couple of the mobile targets rolling build runs).

Disabled tests:

  • System.Security.Cryptography.Rsa.Tests.RSAKeyFileTests.ReadWriteDiminishedDPPrivatePkcs1
  • System.Security.Cryptography.Rsa.Tests.RSAKeyFileTests.ReadEncryptedDiminishedDP_EmptyPassword
  • System.Security.Cryptography.Rsa.Tests.RSAKeyFileTests.DecryptPkcs12PbeTooManyIterations
  • System.Security.Cryptography.Rsa.Tests.RSAKeyFileTests.ReadWriteRsa2048EncryptedPkcs8_Pbes2HighIterations
  • System.Security.Cryptography.Dsa.Tests.DSAKeyFileTests.cs.DecryptPkcs12PbeTooManyIterations
  • System.Security.Cryptography.Dsa.TestsDSAKeyFileTests.cs.ReadWriteDsa1024EncryptedPkcs8_Pbes2HighIterations
  • System.Security.Cryptography.Tests.ECKeyFileTests.DecryptPkcs12PbeTooManyIterations
  • System.Security.Cryptography.Tests.ECKeyFileTests.ReadWriteEc256EncryptedPkcs8_Pbes2HighIterations
  • System.Security.Cryptography.EcDsa.Tests.ECDsaTests_Span

See also #62547.

cc @steveisok @akoeplinger

@ghost ghost added the area-System.Security label Dec 14, 2021
@ghost
Copy link

ghost commented Dec 14, 2021

Tagging subscribers to this area: @dotnet/area-system-security, @vcsjones, @krwq
See info in area-owners.md if you want to be subscribed.

Issue Details

Just blindly disabling several crypto test classes near which crashes have been observed for a while (based on a couple of the mobile targets rolling build runs).

cc @steveisok @akoeplinger

Author: MaximLipnin
Assignees: -
Labels:

area-System.Security

Milestone: -

@MaximLipnin
Copy link
Contributor Author

/azp run runtime-manual

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MaximLipnin MaximLipnin added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Dec 14, 2021
@MaximLipnin MaximLipnin changed the title Disable some suspicious test classes which might lead to OOM crashes on Android emulators and arm64 devices Disable some suspicious tests which might lead to OOM crashes on Android emulators and arm64 devices Dec 14, 2021
@steveisok
Copy link
Member

/azp run runtime-manual

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@steveisok
Copy link
Member

@akoeplinger Ideally, I'd like to run less iterations of tests if we can. Would it be better to skip the crypto suite outright on emulators in order to have max coverage that runs on devices?

@akoeplinger
Copy link
Member

Hm I think it should be possible to reduce the iteration count for the tests, but I haven't looked too deeply. I'm fine with disabling for now.

@MaximLipnin
Copy link
Contributor Author

/azp run runtime-manual

@MaximLipnin MaximLipnin marked this pull request as ready for review December 17, 2021 09:45
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MaximLipnin
Copy link
Contributor Author

There is sill a crash on runtime-manual (Build Android x86 Release AllSubsets_Mono) lane (see log) but it doesn't seem to happen due to the crypto tests as they passed:

4440 I DOTNET  : Tests run: 5418 Passed: 5321 Inconclusive: 0 Failed: 0 Ignored: 79 Skipped: 18
12-17 10:57:10.838  4189  4440 I DOTNET  : ----- Done -----
12-17 10:57:10.838  4189  4207 D DOTNET  : Exit code: 0.
12-17 10:57:10.856  4189  4207 I DOTNET  : MonoRunner finished, return-code=0

@akoeplinger
Copy link
Member

akoeplinger commented Dec 17, 2021

Yeah if you look a little further in that adb log you can see that essentially the whole system crashed and rebooted. That might be still because of the crypto tests e.g. if they still consume too much RAM and the system gets overloaded, but I think we can monitor that in the rolling build and react if necessary.

@akoeplinger akoeplinger removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Dec 17, 2021
@akoeplinger akoeplinger merged commit dda44e1 into dotnet:main Dec 17, 2021
@MaximLipnin MaximLipnin deleted the disable_crypto_tests_on_android branch December 17, 2021 13:11
@ghost ghost locked as resolved and limited conversation to collaborators Jan 16, 2022
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.

3 participants