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

Initial implementation of stateful CBOR writer and reader classes #32803

Merged
merged 33 commits into from
Mar 4, 2020

Conversation

eiriktsarpalis
Copy link
Member

Contributes to #32046 and #32047.

This is an initial, prototype-grade implementation of stateful CBOR reader and writer types.

  • It currently only supports four major types of the format:
    • positive & negative integers,
    • definite-length byte strings and
    • definite-length utf8 encoded strings.
  • The CborWriter and CborReader types are simplistically modeled after the equivalent Asn1 implementations in the same project.
  • I've added a few unit tests using example values from the CBOR spec, as well as a few property-based tests for the sake of introducing entropy in tested values.
  • I've skipped concerns such as xml docs and resource strings for error messages, for now.

@eiriktsarpalis eiriktsarpalis added this to the Future milestone Feb 25, 2020
@eiriktsarpalis eiriktsarpalis modified the milestones: Future, 5.0 Feb 25, 2020
@bartonjs
Copy link
Member

What's here looks reasonable, modulo either testing or removing the TryRead methods for the integer types.

I'm trying to rediscover how to get a test coverage report to include the test assembly, but the obvious things are eluding me.

@bartonjs
Copy link
Member

I'm trying to rediscover how to get a test coverage report to include the test assembly, but the obvious things are eluding me.

The best I've come up with so far:

dotnet msbuild /t:BuildAndTest /p:Coverage=true /p:AssemblyBeingTested=System.Security.Cryptography.Encoding.Tests /p:ForceRunTests=true

Then edit the runtests.bat that's generated and add --include-test-assembly to the call to coverlet.

I don't see an obvious way to get that into the command sequence built from https://github.com/dotnet/runtime/blob/master/eng/testing/coverage.targets; though I would have sworn it used to be. Maybe I'm just not being clever enough.

eiriktsarpalis and others added 2 commits March 3, 2020 17:17
…/CborReader.String.cs

Co-Authored-By: Jeremy Barton <jbarton@microsoft.com>
…/CborReader.String.cs

Co-Authored-By: Jeremy Barton <jbarton@microsoft.com>
@eiriktsarpalis
Copy link
Member Author

Test failures are all System.Reflection and System.Runtime.Loader related.

@eiriktsarpalis
Copy link
Member Author

Then edit the runtests.bat that's generated and add --include-test-assembly to the call to coverlet.

Do you think it might make sense to move to the main project and keep types internal?

@eiriktsarpalis eiriktsarpalis merged commit 9420d6a into dotnet:master Mar 4, 2020
@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.
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants