Refactor main_test.go and add more coverage for main.go

[vepatel]

• main_test.go requires refactoring as almost all tests are covering flags.go,
• main.go requires more test coverage.

[mrajagopal]

So, as a start, would it be fair to say that main_test.go should be renamed flags_test.go with those tests intended for main_test.go moved into such a file?

[mrajagopal]

@vepatel , I have a PR open for the renaming of the file to flags_test.go. I reviewed the coverage for flags.go:

• There are 12 functions in flags.go while flags_test.go has 6 defined
• parseFlags, initialChecks, validateWatchedNamespaces, validationChecks, validateResourceName don't have a test
• validateResourceName doesn't need one as invoked by validateNamespaceNames which has a test anyway
• validateWatchedNamespaces doesn't need one as it invokes validateWatchedNamespaces
• What remain are parseFlags, initialChecks, validationChecks which I think don't need a test either as they are amalgamation/glue procedures and don't take an input

Let me know your thoughts.

[j1m-ryan]

Reopening because the second part is not done yet

main.go requires more test coverage

[mrajagopal]

• There are 28 functions in main.go.
• taking getAndValidateSecret() as an example, there is k8s fake Clientset but this does not match the input to getAndValidateSecret() which requires Cannot use 'kubeClient' (type *"[k8s.io/client-go/kubernetes/fake](http://k8s.io/client-go/kubernetes/fake)".Clientset) as the type *"[k8s.io/client-go/kubernetes](http://k8s.io/client-go/kubernetes)".Clientset
• I suspect some of this boils down to my unfamiliarity with the codebase
• Let me know your/team’s thoughts and anything that I need to understand better to complete this task

main.go	Test Required Yes/No	Notes
func main	No
func createConfigAndKubeClient	No	No return parameter
func kubernetesVersionInfo	No	No return parameter
func validateIngressClass	No	No return parameter
func checkNamespaces	No	No return parameter
func checkNamespaceExists	No	No return parameter
func createCustomClients	Yes
func createPlusClient	Yes
func createTemplateExecutors	Yes
func createNginxManager	Yes	There is NewFakeManager but the intent of this is is different, not what I am thinking of
func getNginxVersionInfo	Yes	But NewFakeManager will need to implement non-empty Version()
func getAppProtectVersionInfo	No?	The function reads /opt/app_protect/VERSION file
func startApAgentsAndPlugins	No	This requires starting an external agent unless we mock that as well
func processDefaultServerSecret	Yes
func processWildcardSecret	Yes	Will need to be adapted to use fake.Clientset
func createGlobalConfigurationValidator	No	This seems to be tested already in globalconfiguration_test.go
func processNginxConfig	No	No return parameter; best tested at the individual module level
func handleTermination	No
func getSocketClient	No
func getAndValidateSecret	Yes	Conflicts with kubernetes.Clientset vs fake.Clientset
func handleTerminationWithAppProtect	No	Creates a golang CHAN waiting for SIGTERM signal
func ready	No	This returns a function
func createManagerAndControllerCollectors	No	This seems like an aggregator function, but we may be better off testing these individually
func createPlusAndLatencyCollectors	Unsure	Quite a busy function and starts a syslog listener
func createHealthProbeEndpoint	No	This calls getAndValidateSecret() locally which shall have a test, also calls RunHealthCheck() as a go routine
func processGlobalConfiguration	Yes
func processConfigMaps	No	This seems more like glue/aggregator function
func updateSelfWithVersionInfo	No	This seems more like glue/aggregator function

[danielnginx]

@mrajagopal We're going to revisit this issue when we have the structured log.