Drop hardcoded raw values in tests #274

New issue

Open

opened 2025-12-28 18:07:38 +00:00 by sami · 3 comments

sami commented

2025-12-28 18:07:38 +00:00

Owner

Originally created by @carpawell on GitHub (Jun 27, 2025).

I'm always frustrated when we test functionality that we do not implement. We can test bbolt, protobuf lib, we can test the whole std lib if we do not trust it. The waste of time will be in accordance.

Current Behavior

Once you change/extend SDK's structures and decide to adopt it in every test file, you open Pandora's box and you start living in signature hell.

Expected Behavior

If we write setters/getters, test setters/getters. If we write marshallers, test marshallers. There is no need to test whether computers can still calculate signatures from some random keys that satisfy us.

Possible Solution

Drop ecdsa test code like nspcc-dev/neofs-sdk-go@673e1845df/bearer/bearer_test.go (L387-L421)

Steps to Reproduce

Update SDK version, have fun.

Context

https://github.com/nspcc-dev/neofs-sdk-go/pull/721

Originally created by @carpawell on GitHub (Jun 27, 2025). I'm always frustrated when we test functionality that we do not implement. We can test bbolt, protobuf lib, we can test the whole std lib if we do not trust it. The waste of time will be in accordance. ## Current Behavior Once you change/extend SDK's structures and decide to adopt it in every test file, you open Pandora's box and you start living in signature hell. ## Expected Behavior If we write setters/getters, test setters/getters. If we write marshallers, test marshallers. There is no need to test whether computers can still calculate signatures from some random keys that satisfy us. ## Possible Solution Drop ecdsa test code like https://github.com/nspcc-dev/neofs-sdk-go/blob/673e1845df8f5cf505bcf10948a38f7aef62219b/bearer/bearer_test.go#L387-L421 ## Steps to Reproduce Update SDK [version](https://github.com/nspcc-dev/neofs-sdk-go/blob/673e1845df8f5cf505bcf10948a38f7aef62219b/version/version.go#L38), have fun. ## Context https://github.com/nspcc-dev/neofs-sdk-go/pull/721

sami added the

test

discussion

labels

2025-12-28 18:07:38 +00:00

sami commented

2025-12-28 18:07:39 +00:00

Author

Owner

@roman-khimov commented on GitHub (Jun 27, 2025):

Some hardcoded values are very much useful, we need to make sure that exact known binaries are always processed correctly, because marshal/unmarshal can have some symmetric bugs that make tests green, but then it'll all fail with real objects. Also, binaries allow to easier testing of protocol violation handling in some cases. But:

I'd not expect any of these tests to fail on SDK upgrade, this just looks like a test problem
maybe we have a bit too much of hexes in our code

So if there are bugs, let's fix them. If there are some redundant tests, let's discuss and refactor/drop them. But dropping all binaries just because they're binaries is not appropriate.

@roman-khimov commented on GitHub (Jun 27, 2025): _Some_ hardcoded values are very much useful, we need to make sure that exact known binaries are always processed correctly, because marshal/unmarshal can have some symmetric bugs that make tests green, but then it'll all fail with real objects. Also, binaries allow to easier testing of protocol violation handling in some cases. But: * I'd not expect any of these tests to fail on SDK upgrade, this just looks like a test problem * maybe we have a bit too much of hexes in our code So if there are bugs, let's fix them. If there are some redundant tests, let's discuss and refactor/drop them. But dropping all binaries just because they're binaries is not appropriate.

sami commented

2025-12-28 18:07:39 +00:00

Author

Owner

@cthulhu-rider commented on GitHub (Jul 8, 2025):

If we write marshallers, test marshallers

how to test marshaler? Make sure it produces exact binary

@cthulhu-rider commented on GitHub (Jul 8, 2025): > If we write marshallers, test marshallers how to test marshaler? Make sure it produces exact binary

sami commented

2025-12-28 18:07:39 +00:00

Author

Owner

@carpawell commented on GitHub (Jul 8, 2025):

But dropping all binaries just because they're binaries is not appropriate.

I have not said that. I do not understand how hardcoded signatures + commented public keys help us even a little. I do not understand why "marshal + sign" was important then, but now, when there is one more field, this field is not required, you should not use it in tests (but if the field has "non-zero zero" value, any signature will break anyway) OR a dev dies recalculating all the signatures of the new field. Even if we are recalculating signatures, you do not check if they are ok, you just hardcode new values that you just received, and that is your new base from now.

how to test marshaler? Make sure it produces exact binary

Does it require signing these binaries?

@carpawell commented on GitHub (Jul 8, 2025): > But dropping all binaries just because they're binaries is not appropriate. I have not said that. I do not understand how hardcoded signatures + commented public keys help us even a little. I do not understand why "marshal + sign" was important then, but now, when there is one more field, this field is not required, you should not use it in tests (but if the field has "non-zero zero" value, any signature will break anyway) OR a dev dies recalculating all the signatures of the new field. Even if we are recalculating signatures, you do not check if they are ok, you just hardcode new values that you just received, and that is your new base from now. > how to test marshaler? Make sure it produces exact binary Does it require signing these binaries?