Skip to content

Chore: Make TestOrchestration subtests follow standard testing style #159

@meling

Description

@meling

This should wait until ongoing PRs that touch this test have been merged...

Instead of:

func TestOrchestration(t *testing.T) {
	run := func(t *testing.T, consensusImpl string, crypto string, mods []string, byzantine string) {

We should write:

func TestOrchestration(t *testing.T) {
	tests := []struct {
		consensus string
		crypto    string
		byzantine string
		mods      []string
	}{
		{consensus: "chainedhotstuff", crypto: "ecdsa", byzantine: "", mods: nil},
		{consensus: "chainedhotstuff", crypto: "eddsa", byzantine: "", mods: nil},
		{consensus: "chainedhotstuff", crypto: "bls12", byzantine: "", mods: nil},
		{consensus: "fasthotstuff", crypto: "ecdsa", byzantine: "", mods: nil},
		{consensus: "fasthotstuff", crypto: "eddsa", byzantine: "", mods: nil},
		{consensus: "fasthotstuff", crypto: "bls12", byzantine: "", mods: nil},
		{consensus: "simplehotstuff", crypto: "ecdsa", byzantine: "", mods: nil},
		{consensus: "simplehotstuff", crypto: "eddsa", byzantine: "", mods: nil},
		{consensus: "simplehotstuff", crypto: "bls12", byzantine: "", mods: nil},
		{consensus: "chainedhotstuff", crypto: "ecdsa", byzantine: "fork:1", mods: nil},
		{consensus: "chainedhotstuff", crypto: "ecdsa", byzantine: "silence:1", mods: nil},
		{consensus: "chainedhotstuff", crypto: "ecdsa", byzantine: "", mods: []string{"handel"}},
		{consensus: "chainedhotstuff", crypto: "bls12", byzantine: "", mods: []string{"handel"}},
	}
	for _, tt := range tests {
		t.Run(test.Name([]string{"consensus", "crypto", "byzantine", "mods"}, tt.consensus, tt.crypto, tt.byzantine), func(t *testing.T) {
			// content of run function
		})
	}

Here is the implementation of test.Name (which could be placed in internal/test package):

// Name returns the test name based on the provided fields and values.
func Name(fields []string, values ...any) string {
	if len(fields) != len(values) {
		panic("fields and values must have the same length")
	}
	b := strings.Builder{}
	for i, f := range fields {
		v := values[i]
		switch x := v.(type) {
		case []string:
			if x == nil {
				b.WriteString(fmt.Sprintf("/%s=<nil>", f))
			} else {
				b.WriteString(fmt.Sprintf("/%s=%v", f, x))
			}
		case string:
			if x != "" {
				b.WriteString(fmt.Sprintf("/%s=%s", f, v))
			}
		case uint64:
			if x != 0 {
				b.WriteString(fmt.Sprintf("/%s=%d", f, v))
			}
		case int:
			if x != 0 {
				b.WriteString(fmt.Sprintf("/%s=%d", f, v))
			}
		case bool:
			if x {
				b.WriteString(fmt.Sprintf("/%s", f))
			}
		default:
			b.WriteString(fmt.Sprintf("/%s=%v", f, v))
		}
	}
	return b.String()
}

Metadata

Metadata

Assignees

Labels

cleanupImprove readability, design patterns, and simplification

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions