Skip to content

Consolidate cryptography tests to System.Security.Cryptography #66338

Open

Description

@vcsjones

With #55690 merged, the implementations of the S.S.Cryptography assemblies have been consolidated in to a single in-box assembly, where possible.

The unit tests for these are still in their separate projects however, such as S.S.C.Algorithms, S.S.C.Cng, etc.

This issue is to track consolidating them under System.Security.Cryptography. Note that this is not as simple as moving the files and adjusting namespaces. We made heavy use of partials to test different implementations of algorithms (internal vs public CNG for example). The partial approach won't work. The current solution is to refactor to a "driver" base type and test each implementation as a derived type.

Activity

  1. ghost added
    untriagedNew issue has not been triaged by the area owner
    on Mar 8, 2022
  2. bartonjs commented on Mar 8, 2022

    @bartonjs
    Member

    Huh, thought I'd opened this already. But, I hadn't. Thanks 😄.

    https://github.com/bartonjs/runtime/commits/crypto_test_model has my first-draft thoughts here. They seemed to be fruitful, but certainly aren't a commitment.

  3. vcsjones commented on Mar 14, 2022

    @vcsjones
    MemberAuthor

    We could move X509Certificates pretty easily, if not for:

    [assembly: ActiveIssue("https://github.com/dotnet/runtime/issues/57810", typeof(PlatformDetection), nameof(PlatformDetection.IsMonoRuntime), nameof(PlatformDetection.IsMariner))]

    I don't think we want to disable all of S.S.Cryptography tests on Mariner. But do we actually need to disable X509Certificates on Mariner? According to #57810 (comment), that ActiveIssue didn't stop the Mariner crashes.

    I think the given options are:

    1. Block moving X509Certificate tests until System.Security.Cryptography.X509Certificates.Tests crashing on Mariner OS #57810 is solved.
    2. Put ActiveIssue on every single X509Certificates class, and future classes.
    3. Move all of the tests anyway and remove the assembly-level ActiveIssue since prior comment indicated it did not help. If that is true, then what is being done for Mariner? Are tests running, or did we disable them some other way?

    cc @ViktorHofer in case you have any insight on the X509Certificate tests in Mariner.

  4. removed
    untriagedNew issue has not been triaged by the area owner
    on Jul 6, 2022
  5. added this to the Future milestone on Jul 6, 2022
  6. bartonjs commented on Jul 6, 2022

    @bartonjs
    Member

    Moving to the Future/Indefinite milestone because it's just code cleanup and our own pain, nothing impacting customers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions