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

Use separate key instances for span/array/array+offset test classes #34199

Merged
merged 1 commit into from
Mar 28, 2020

Conversation

bartonjs
Copy link
Member

Because the key object generation was done in the algorithm-specific base class,
the triplet of interface types was using the key instances in parallel.

By moving the static variable (and initialization thereof) to each of the derived
classes, the key objects are unique per class, which matches the test parallelism.

Making the classes be part of the same test collection would also solve this problem,
which would save on a few random keygens, but would likely overall take more
time due to the number of tests that would be moved to sequential execution.

Fixes #34045.

Because the key object generation was done in the algorithm-specific base class,
the triplet of interface types was using the key instances in parallel.

By moving the static variable (and initialization thereof) to each of the derived
classes, the key objects are unique per class, which matches the test parallelism.

Making the classes be part of the same test collection would also solve this problem,
which would save on a few random keygens, but would likely overall take more
time due to the number of tests that would be moved to sequential execution.
@bartonjs bartonjs added area-System.Security test-bug Problem in test source code (most likely) labels Mar 27, 2020
@bartonjs bartonjs added this to the 5.0 milestone Mar 27, 2020
@bartonjs bartonjs requested review from krwq and stephentoub March 27, 2020 15:51
@ghost
Copy link

ghost commented Mar 27, 2020

Hello @bartonjs!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

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 (@msftbot) and give me an instruction to get started! Learn more here.

@jkotas jkotas merged commit 7557b78 into dotnet:master Mar 28, 2020
@bartonjs bartonjs deleted the no_parallel_key_use branch March 29, 2020 19:16
@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Security test-bug Problem in test source code (most likely)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cryptograph.Csp tests are failing intermittently
3 participants