vtctl.generateShardRanges -> key.GenerateShardRanges

[guidoiaquinti]

Signed-off-by: Guido Iaquinti giaquinti@slack-corp.com

Description

Export vtctl.generateShardRanges to key.GenerateShardRanges so it can be used also in other places (I'm not sure key is the best package where to put it but I didn't find a better one).

Checklist

• Tests were added or are not required
• Documentation was added or is not required

Deployment Notes

[guidoiaquinti]

The error has been renamed

[guidoiaquinti]

Before it was require.Error(t, err) but I don't think this was really needed (and we can avoid importing require)

[ajm188]

We actually want the behavior of require in the cases where tt.wantErr=false in that it will stop execution of the test if the assertion fails (this is the main difference between the assert and require packages. It makes the intent of the test assertion much clearer in my view

[guidoiaquinti]

Package require implements the same assertions as the assert package but stops test execution when a test fails.

I don't see a big difference between the two but if you think it's cleaner I revert my change (see next commit).

[guidoiaquinti]

Ditto

[ajm188]

Just the one comment about require and then lgtm

[ajm188]

We actually want the behavior of require in the cases where tt.wantErr=false in that it will stop execution of the test if the assertion fails (this is the main difference between the assert and require packages. It makes the intent of the test assertion much clearer in my view

[ajm188]

you no longer need the else here (this is the benefit of require)

this should read:

if tt.wantErr {
    assert.Error(t, err)
    return
}

require.NoError(t, err)
assert.Equal(t, got, tt.want)

[guidoiaquinti]

Fair but I've blindly copy pasted what was there before:

vitess/go/vt/vtctl/vtctl_test.go

Lines 59 to 70 in 7c532a5

	for _, tt := range tests {
	t.Run(tt.name, func(t *testing.T) {
	got, err := generateShardRanges(tt.args.shards)
	if tt.wantErr {
	require.Error(t, err)
	} else {
	require.NoError(t, err)
	}
	assert.Equal(t, got, tt.want)
	})
	}

[ajm188]

🤦 haha sorry, and thank you for cleaning up my original mistake