influx client needs more test coverage

[otoolep]

This issue was exposed by PR #2306. main_test.go in influx should not be importing main packages, as this is not a valid operation.

Relevant links:

#2306
golang/go#4210

[otoolep]

@corylanou

[beckettsean]

@otoolep @corylanou fix or close?

[corylanou]

I think keep it open. The real fix is to move most of the actual "work" to another package (or sub package) and the main package becomes just a shell that basically calls into the sub package with a New and then Runs it, etc. This will allow us to test without a main package and put these tests back in place.

You can look at the cmd/influxd/run package to get an idea about how we are outside of main from cmd/influxd

I would say this is a beginner to medium task if the community wanted to take a pass at it.

[pires]

I'm willing to own this after #4711 #4710 #4704 and #4702 are either merged or closed. I'd like to:

• Refactor history
  • Don't write to file at all times but only when commands are valid ones
  • Don't read history from file but from memory (use WriteHistory(bytes.Buffer))
• Improve command parsing including...
• Use parser to validate (at least some) statements before sending to server

[corylanou]

That sounds awesome! +1 from me on doing those things. I'll work on getting the rest of those PR's reviewed today as well so we can get them merged.

[pires]

Attention: this issue is placeholder for ideas, suggestions or comments related to the aforementioned refactor. I will probably move it to the proper issue when #4711 #4710 #4704 and #4702 are reviewed.

As per @corylanou comments:

It would be nice in the test to understand if these commands selected the correct "code branch". I understand it isn't possible with the way the current method is written. I'm only bringing it up as it may be something to think about when you refactor the cli main package going forward.

[corylanou]

@pires ok, I reviewed them. Notified the core team we need one more +1 from another core team member and then we will merge. Thanks for all the hard work!

[pires]

Thank you @corylanou.

[jsternberg]

I'm closing this since it doesn't have any specific value such as "achieve 80% code coverage". I'm also closing all client related issues and taking those into account for a new client that will be at influxdata/influxdb-client when it's done.