Siac test structure
We should create some structure and standards around how siac tests should be run.
This issue will have multiple MRs during refactoring and addressing its sub-issues mentioned below.
Created from discussion here:
The following discussion from !4322 (merged) should be addressed:
q: What does this accomplish? And is it okay to set a global in a test? Will it not interfere with other tests?
Yea we don't have a standard around siac tests yet since all the other tests are unit tests that don't hit the API.
Maybe the right thing to do is for this MR to create a helper function that will be used moving forward. Since the siac tests should just need an initialized client we could create a newTestingClient(testname string) that does what you did here with creating a siatest node and setting httpClient to it.
I think the one restriction will be that siac tests will not be able to be run in parallel then, as you will get data races for setting the global variable.
Any option then might be that in addition to the test helper method we then structure siac tests as TestGroups. So TestSkykeyCommands is the test group and then it will call a sub test on each of the sky key commands.
That would help use ensure that tests are not being run in parallel, we can set the httpClient once on init of the test group, and we can easily extend the testing with new commands. That would also be a nice structure for the other packages to follow.
From another discussion:
rootCmd) is the more problematic global variable that should be refactored next. I took a look at the tests in
cobra. This file https://github.com/spf13/cobra/blob/master/command_test.go has good examples of what we can do. They define a helper method
executeCommandthat passes args and flags into a
cobra.Commandand stores the STDOUT/STDERR in a buffer. If we shifted our tests to something like this we can run the cmd functions directly and pass in flags/args the same way they are passed in production. This means we wouldn't have to define all these helper functions either. It also means we can test the structure of
executeCommandI would think we still need to pass in a client to use other than the global client so that tests can be run in parallel and ensure they are initialized as intended. In which case this MR is still a step in that direction of updating the siac functions to accept the client. I agree it would be interesting as a follow up MR to investigate that
executeCommandhelper so that the siac tests are more end to end.