R4R: Fix Non-Deterministic Test Coverage

[alexanderbez]

Took a look at about 15-20 pages worth of CircleCI builds and looked at the ones that failed due to test_cover. From what I gathered tests fail due to missing but expected Tendermint WAL files OR an address already being bound in LCD tests. Dug through the code and found that the only address that does not get modified on each LCD test is the GRPC address, coincidentally that is what I see in the logs: listen tcp 0.0.0.0:36658: bind: address already in use.

• Cleaned up LCD test helpers (godocs, formatting, etc...)
• Generate GRPC address in GetConfig

Note: The WAL issue will still need to be addressed, but I'm pretty sure that is TM side?

closes: #1675

---

• Linked to github-issue with discussion and accepted design
• Updated all relevant documentation (docs/)
• Updated all relevant code comments
• Wrote tests
• Added entries in PENDING.md that include links to the relevant issue or PR that most accurately describes the change.
• Updated cmd/gaia and examples/

---

For Admin Use:

• Added appropriate labels to PR (ex. wip, ready-for-review, docs)
• Reviewers Assigned
• Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

[codecov]

Codecov Report

Merging #1850 into develop will increase coverage by 0.02%.
The diff coverage is 93.75%.

@@             Coverage Diff             @@
##           develop    #1850      +/-   ##
===========================================
+ Coverage     63.5%   63.52%   +0.02%     
===========================================
  Files          118      118              
  Lines         7006     7016      +10     
===========================================
+ Hits          4449     4457       +8     
- Misses        2301     2302       +1     
- Partials       256      257       +1

[ValarDragon]

utACK! Good catch :).

While not super important for small PR's, things are much easier / faster to review if we separate out functionality changes from refactors. (i.e. renaming getkeybase)

[ValarDragon]

Does this close #1639 as well?

[alexanderbez]

@ValarDragon

While not super important for small PR's, things are much easier / faster to review if we separate out functionality changes from refactors. (i.e. renaming getkeybase)

Fair enough, but sometimes I just cannot help myself 😆 , although I will try to limit it to just documentation improvements.

Does this close #1639 as well?

Not exactly. However, I have not seen that error in builds that I've looked at (which was a lot). I think it might be safe to close #1639 for now.

[rigelrozanski]

lol I don't like this (fine) but I may switch back at some point - why use 4 lines when you could use 2! - stylistically I'd think it's cleaner to only use var blocks for global variables - or under certain very specific situations where there a whole bunch of variables in a in a function (maybe 5+) - yeah the extra indentation is a bit confusing because in funcs typically means your in an If/switch/for

[alexanderbez]

Hmmm, I suppose we have stylistic differences. I see the above as much cleaner and easier to read. Yes, granted, it's two extra lines, but Go is naturally a verbose language. However, I will limit such refactors in future work.

[rigelrozanski]

not a sentence

[rigelrozanski]

good catch - next time seperate out functionality changes from cleanup/refactors. Thanks! (Also see my comments) - still want to merge though