#5 add cli

[XioZ]

• Implemented a CLI controller to start/stop a mino instance
• Closing Add Controller  #5

[ineiti]

Looks good - just some test optimizations.

[ineiti]

Simple name change to make it clearer what the storage is for. Else it looks good.

[ineiti]

And other tests - ChatGPT proposes the following change, what do you think?

func TestController_OnStart(t *testing.T) {
	ctrl, flags, inj, stop := setupTest(t, "/ip4/0.0.0.0/tcp/8000/ws", "/dns4/p2p-1.c4dt.dela.org/tcp/443/wss")
	defer stop()

	err := ctrl.OnStart(flags, inj)
	require.NoError(t, err)
	var m *minows
	err = inj.Resolve(&m)
	require.NoError(t, err)
}

// ... Other tests using `setupTest` ...

func TestController_InvalidPublic(t *testing.T) {
	ctrl, flags, inj, _ := setupTest(t, "/ip4/0.0.0.0/tcp/8000/ws", "invalid")

	err := ctrl.OnStart(flags, inj)
	require.Error(t, err)
}

func TestController_OnStop(t *testing.T) {
	ctrl, flags, inj, _ := setupTest(t, "/ip4/0.0.0.0/tcp/8000/ws", "/dns4/p2p-1.c4dt.dela.org/tcp/443/wss")

	err := ctrl.OnStart(flags, inj)
	require.NoError(t, err)

	err = ctrl.OnStop(inj)
	require.NoError(t, err)
}

// ... Other tests ...

// `TestController_MissingDependency` doesn't need `stopFunc`, but common setup is still useful.
func TestController_MissingDependency(t *testing.T) {
	ctrl, flags, inj, _ := setupTest(t, "/ip4/0.0.0.0/tcp/8000/ws", "/dns4/p2p-1.c4dt.dela.org/tcp/443/wss")

	err := ctrl.OnStart(flags, inj)
	require.NoError(t, err)

	err = ctrl.OnStop(node.NewInjector())
	require.Error(t, err)
}

type mockFlags struct {
	mock.Mock
}

// Helper function for common setup
func setupTest(t *testing.T, listenValue string, publicValue string) (ctrl *controller.Controller, flags *mockFlags, inj *node.Injector, stopFunc func()) {
	flags = new(mockFlags)
	flags.On("String", "listen").Return(listenValue)
	flags.On("String", "public").Return(publicValue)
	inj = node.NewInjector()
	inj.Inject(fake.NewInMemoryDB())

	ctrl, stopFunc = mustCreateController(t, inj)

	return ctrl, flags, inj, stopFunc
}

[XioZ]

i'm not a big fan of overengineering tests because 1) tests need to change accordingly when requirements change. if i need to modify 1 test setup that requires refactoring setupTest, that affects all other tests that use it unnecessarily 2) having the setup code all in the test itself makes the test more readable (without scrolling). however, i can make this change if you prefer brevity.

[ineiti]

If it's two or three tests with the same setup, I do agree that it's not worth having a common setup. But here it is five tests, so I think it's worth it.

[XioZ]

changed