refactor Echo server startup to allow data race free access to listener address

[aldas]

Refactor Echo server startup to allow data race free access to listener address.

This is problem that I have had with one of my apps - it is hard to create FAST test that spins up echo server and does minimal waiting for it. Problem is that to see if server is up and to use it you need to know on what port its listener was bind. but server startup and checking are done in different goroutines as serve start is blocking function call and when you try to acces listener you will have data race failure.

another problem - there are plenty of tests in echo that are doing 200ms waits for server startup and this adds up (about ~2sec excess at the moment)

NB: Echo has many methods to start server. Two instances of listener and server but only one is used when running server - it is confusing.

		Server           *http.Server
		TLSServer        *http.Server
		Listener         net.Listener
		TLSListener      net.Listener

@lammel @pafuent what are your thoughts? maybe someone has better/cleaner ideas for tests/data race

note: work in progress

[codecov]

Codecov Report

Merging #1735 (d18c040) into master (b065180) will increase coverage by 0.79%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1735      +/-   ##
==========================================
+ Coverage   85.87%   86.66%   +0.79%     
==========================================
  Files          29       29              
  Lines        2018     2063      +45     
==========================================
+ Hits         1733     1788      +55     
+ Misses        180      175       -5     
+ Partials      105      100       -5     

Impacted Files	Coverage Δ
echo.go	91.48% <100.00%> (+5.08%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b065180...d18c040. Read the comment docs.

[lammel]

There already where some questions request to know when echo startup is completed.
As we can take advantage of that also for our tests I really think this is a good thing to have.

If we need to change interfaces or documented functions for startup we should wait for v5, but the current proposal works with external changes so far.

[aldas]

I did not intend to change API in breakable way. Added only 2 methods ListenerAddr and TLSListenerAddr to have access to listener address.

But for future (v5+) maybe there should be task/issue to investigate/refactor Echo internals a little. For example:

• duplication of Listerner and Server instance in Echo struct is strange - currently Start* methods only serve single http.Server.
• maybe instead of holding pointer to Listener/Server instance inside Echo there should be channel for signalling server to shut down. For me it seems that Listener/Server are there only for Shutdown purposes.
• maybe refactor Echo Start* methods so that Echo can be attached to multiple Listeners (binded to many ports ala 80+81+443) and Shutdown signals attached servers to shut down.
• comment all fields on Echo struct

[lammel]

I did not intend to change API in breakable way. Added only 2 methods ListenerAddr and TLSListenerAddr to have access to listener address.

Agreed. If we can prepare the infrastructure without modifying the interface we can merge it for v4.
For v5 let us make a new (like more generic) ticket to "improve echo startup and configuration" or something similar (with your suggestions, that pretty much covers it).

@aldas As you did all the preliminary work maybe it makes sense that you open the v5 issue too (if you want).

But for future (v5+) maybe there should be task/issue to investigate/refactor Echo internals a little. For example:

• duplication of Listerner and Server instance in Echo struct is strange - currently Start* methods only serve single http.Server.
• maybe instead of holding pointer to Listener/Server instance inside Echo there should be channel for signalling server to shut down. For me it seems that Listener/Server are there only for Shutdown purposes.

For an external interface using channels is not encouraged, so I guess we should also use callback functions here to wrap the channel. Seems like this is easier to understand and is in line with our other handlers.
It should be available for startup and shutdown too.

[lammel]

Test runtime is reduced from ~2.9s to ~0.6s now.

Before:

rl@slimboy> go test -count=1 ./...
ok  	github.com/labstack/echo/v4	2.914s
ok  	github.com/labstack/echo/v4/middleware	0.027s

Now (with this PR):

rl@slimboy> go test -count=1 ./...
ok  	github.com/labstack/echo/v4	0.579s
ok  	github.com/labstack/echo/v4/middleware	0.028s

This PR is going to be merged now. A follow up for v5 will be created. Thanks @aldas