Fix all remaining System.Security.Cryptography.Algorithms test suite crashes. #48432
Conversation
…crashes. With this + PR dotnet#48348, the test suite runs to completion and reports the following results: > Tests run: 2527 Passed: 1285 Inconclusive: 0 Failed: 1227 Ignored: 7 So we've got a ways to go to get things passing, but now we can easily track it at least for this assembly.
jkoritzinsky
requested review from
steveisok,
EgorBo,
AaronRobinsonMSFT and
elinor-fung
|
Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq, @GrabYourPitchforks Issue DetailsWith this + PR #48348, the test suite runs to completion and reports the following results:
So we've got a ways to go to get things passing, but now we can easily track it at least for this assembly.
|
| @@ -111,7 +111,7 @@ private void ImportKey(ReadOnlySpan<byte> key) | |||
|
|
|||
| if (plaintextBytesWritten != plaintext.Length) | |||
| { | |||
| Debug.Fail($"GCM decrypt wrote {plaintextBytesWritten} of {plaintext.Length} bytes."); | |||
| // Debug.Fail($"GCM decrypt wrote {plaintextBytesWritten} of {plaintext.Length} bytes."); | |||
There was a problem hiding this comment.
I'll work on trying to isolate and fix this issue, but in the meantime this is the easiest way to keep this Debug.Fail method from taking down the test process.
There was a problem hiding this comment.
Unlike ECB and CBC, GCM produces one byte of output for one byte of input. Goodness isn't happening if this assert fails (and tests seemingly shouldn't be passing, unless they got the data from elsewhere having depended on this assert)
There was a problem hiding this comment.
If this is needed, please don't merge without creating an issue to put it back.
There was a problem hiding this comment.
Oh the tests aren’t passing. But this commented out assert allows them to run and fail and have XUnit record the issue.
There was a problem hiding this comment.
They fail from the exception that immediately follows this assert.
There was a problem hiding this comment.
Tracking issue for re-enabling the assert is here: #48471
Is this suite in some state where certain classes/groupings of tests pass now (with your other PR)? I added |
src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_evp_cipher.c
Show resolved
Hide resolved
src/libraries/Native/Unix/System.Security.Cryptography.Native.Android/pal_evp_cipher.c
Show resolved
Hide resolved
|
There's some suites that fully pass and others that are partial. I'll try to pull together a list for the issue based on this PR. |
jkoritzinsky
force-pushed
the
android-fix-algos-test-crash
branch
from
ec3dc47
to
cc5927e
Compare
jkoritzinsky
force-pushed
the
android-fix-algos-test-crash
branch
from
cc5927e
to
d6decdd
Compare
|
@elinor-fung I've updated the table with the list of test suites and added a link to the program I used to get the list so we can use it for the other assemblies and as we complete the test suites. |
|
Hello @jkoritzinsky! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
ghost
merged commit 
ghost
locked as resolved and limited conversation to collaborators
With this + PR #48348, the test suite runs to completion and reports the following results:
So we've got a ways to go to get things passing, but now we can easily track it at least for this assembly.