All test stuff #88

Merged
ABastionOfSanity merged 29 commits from all_test_stuff into main 2023-01-30 21:59:14 +00:00
2 changed files with 3 additions and 2 deletions
Showing only changes of commit a1ad1e1d35 - Show all commits

View File

@ -24,7 +24,8 @@ func TestAndValidateCIDGeneration(t *testing.T) {
for _, tc := range testCases { for _, tc := range testCases {
deprecatedAndCorrect, _ := CIDFromJSONBytes([]byte(tc.content)) deprecatedAndCorrect, _ := CIDFromJSONBytes([]byte(tc.content))
newImpl, _ := CIDFromJSONBytesUsingIpldPrime([]byte(tc.content)) newImpl, err := CIDFromJSONBytesUsingIpldPrime([]byte(tc.content))
require.NoError(t, err)
require.Equal(t, deprecatedAndCorrect, newImpl, tc.name) require.Equal(t, deprecatedAndCorrect, newImpl, tc.name)
Review

We should either require.NoError() the errors returned by these functions or make sure the returned result isn't nil/empty, because if both functions were to error out and return a nil/empty result it would evaluate them as equal and the test would pass.

We should either `require.NoError()` the errors returned by these functions or make sure the returned result isn't nil/empty, because if both functions were to error out and return a nil/empty result it would evaluate them as equal and the test would pass.
Review

Also, ideally we'd have a static fixture for what the expected CID is for some input content- so that if both functions are updated in the same change set we can be certain we didn't break them but in the same way such that this test still passes.

But, since we will be getting rid of CIDFromJSONBytes soon we shouldn't do that here. Once we get rid of CIDFromJSONBytes, we will have to test against a static fixture.

Also, ideally we'd have a static fixture for what the expected CID is for some input content- so that if both functions are updated in the same change set we can be certain we didn't break them but in the same way such that this test still passes. But, since we will be getting rid of `CIDFromJSONBytes` soon we shouldn't do that here. Once we get rid of `CIDFromJSONBytes`, we will have to test against a static fixture.
} }
} }