Unit tests - scorer/registry

[nathannaveen]

• Added unit tests for scorer/registry

Signed-off-by: nathannaveen 42319948+nathannaveen@users.noreply.github.com

[calebbrown]

The global registry shouldn't be used as the state is not reset between tests.

Instead a new registry should be created for each test iteration.

[nathannaveen]

I re-set GlobalRegistry to NewRegistry() every test so we don’t run into this problem.

I can't create a new registry for every test because I am calling NewAlgorithm(), and NewAlgorithm()'s return is based on GlobalRegistry. If I create a new registry for every test, GlobalRegistry will always be empty when calling NewAlgorithm().

[calebbrown]

A better approach to this test method might be to:

1. populate the registry with one or two entries before the test loop (I suggest having a factory that always works and stores inputs, and a factory that always returns an error).
2. define tests that call NewAlgorithm on the registry.

i.e:

func TestNewAlgorithm(t *testing.T) {
  r := NewRegistry()
  r.Register("test", func(...) { ... })

  tests := []struct{
    // ...
  }{
    // ...
  }
  for _, test := range tests {
    t.Run(...)
  }

[nathannaveen]

I am registering GlobalRegistry at the top. I am also resetting GlobalRegistry using t.Cleanup() so as to not pollute the global variable.

If there is a better way to do this, could you please explain it to me with code examples? I don't mind you adding commits to this PR if that makes it easier.

Thanks

[calebbrown]

Using a custom registry is a good idea as it avoids side-effect and removes the need to do cleanups.

[nathannaveen]

Sorry, I don't understand what you mean by "custom registry", could you please explain and give some examples? It's been a while since I worked on this PR :)

[calebbrown]

When I say custom registry I mean the one returned by r := NewRegistry(), rather than using the GlobalRegistry.

By creating a new Registry in each test function, you avoid the need to run a cleanup.

[nathannaveen]

I can't use NewRegistry() because Register() and NewAlgorithm() are based on GlobalRegistry meaning that they won't run properly without setting GlobalRegistry.

For example, if we used r := NewRegistry() and then registered r using r.Register(). GlobalRegistry would be empty when calling NewAlgorithm().

[nathannaveen]

@calebbrown, a friendly ping!