Add sk-cbor boundary and ordering tests

[ivmat]

if youd rather not add any of these test cases, we simply drop this PR. I take responsibility for this PR and will answer review comments myself.

Disclamer: this pr is prepared by AI and is part of my wider campaignt to promote a format and protocol ive been developing: acceptance format. For now its shown use as a good internal tracking tool of semantic code coverage while also "testing" the test cases by giving only weight to test cases that can be shown to be able to fail. The goal of protocl is wider: a consumer-producer "contract" (legal,monetary not part of it) - you can see in the repo.

How the gaps were found: acceptance file for the sk-cbor crate (whole public API, at the same source as develop). Its mutation claims M001, M021, M022, M024, M035–M037 and M075–M077 correspond to this PR's tests; the list of surviving mutations shows what each one led to, including the ones this PR does not propose.

---

Details (AI-generated)

Add sk-cbor unit tests for u64::MAX decoding, nesting limits through the public read, write, read_nested and write_nested functions, and value ordering: 6 new tests, and new cases in test_read_unsigned and test_value_ordering. Library code is unchanged.

test_read_unsigned gets the RFC 8949 Appendix A u64::MAX case; its cases stopped at i64::MAX. The cases now use cbor_unsigned!, because u64::MAX does not fit cbor_int!; for the existing cases both macros build the same value.

The default-limit tests check the documented limit of 127 on read and write: 127 nested arrays pass and 128 fail with TooMuchNesting.

With None, read_nested decodes 128 nested arrays to the expected value and write_nested encodes them to the expected bytes. 128 is one level more than any Some limit allows (i8::MAX = 127). This follows #373, which keeps None as the way to skip the nesting check.

Existing tests already cover the limit logic: test_read_too_much_nesting_error, test_write_single_levels, test_write_nested_map_levels, test_write_unbalanced_nested_containers and test_write_overly_nested. These exercise the crate-private Reader and Writer; test_write_nested_map_levels and test_write_overly_nested already check successful writes with None.

test_value_ordering gets three cases: equal-length arrays and maps whose first element or pair matches, and tags with the same tag number but different contents.

One open point: the tag tests check that Some(0) rejects and Some(1) accepts a tag that contains an integer. This pins that a tag uses one nesting level, as the code does now; the docs do not say so. Please confirm, or the two tag tests can be dropped.

[coveralls]

coverage: 97.239%. remained the same — ivmat:add-sk-cbor-boundary-tests into google:develop

[kaczmarczyck]

Coverage didn't improve, but tests make sense.
Approved outside of small style comments.

[ivmat]

Coverage didn't improve, but tests make sense. Approved outside of small style comments.

true - but the code coverage is not the point of this new format -- semantical coverage is. consider specification, enumerate it as requirements (plus any additional requirement such as "dont panic on any input"). so if we enureate each requimente, we can confirm if the requirement is checked or not (via test ,formal verfication ect) - and even more we can confirm if the verification actually verfifes (by mutating code with a fault introdcued). so only really hard thing is to enumerate all rquirements, then adding a test and checking if it really covers something can be left to llms. i will address all other comments.

In this particular case, one requirement and its parts:

Requirement: RFC 8949 §3.1, major type 0: “An unsigned integer in the range 0..2^(64)-1 inclusive.” Enumerated as RFC-UINT.
Acceptance: the row for RFC-UINT names the test that checks the requirement.
Fault: mutant M001 makes the reader clear the top bit of the 64-bit argument (size_value & i64::MAX as u64). As a result, 0x1B FF FF FF FF FF FF FF FF decodes as 9223372036854775807, not 18446744073709551615.
Existing tests with the fault in: all 78 unit tests and all 5 doc tests still pass. So the requirement is listed but no test actually checks it.
New test with the fault in: test_read_unsigned_max from this PR fails with left: 9223372036854775807, right: 18446744073709551615.

Line coverage cannot show this gap: the faulty line already runs in the existing tests. The same chain for every requirement: all requirements, all acceptance rows, and the 19 faults the existing tests miss, with what was done about each one.

[kaczmarczyck]

My response could have been more verbose: I agree that test coverage is only one metric, and these tests add value. My message was a response for anyone who sees the coveralls comment and wonders about it.

[ivmat]

My response could have been more verbose: I agree that test coverage is only one metric, and these tests add value. My message was a response for anyone who sees the coveralls comment and wonders about it.

thank you for clarification. also wondering if you would be interested in 2 more items found, i can open a PR if so:

4 deferred tests: https://github.com/ivmat/OpenSK/blob/9fbafbd9120221aa656ecbcdf9cb2b60ef08d56a/acceptance/sk-cbor/acceptance/deferred-tests.patch
Documentation fix: https://github.com/ivmat/OpenSK/blob/9fbafbd9120221aa656ecbcdf9cb2b60ef08d56a/acceptance/sk-cbor/acceptance/upstream-doc.patch

[kaczmarczyck]

I consider the CBOR library done at the moment. I'd take the documentation fixes, but the long unit test list is more work than useful from a quick glance.

[ivmat]

thanks for checking! i guess no need to open anything, since doc update is too small