Fix LoadHTML* tests #1559
Fix LoadHTML* tests #1559
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1559 +/- ##
==========================================
- Coverage 99.06% 98.79% -0.27%
==========================================
Files 39 39
Lines 1915 1915
==========================================
- Hits 1897 1892 -5
- Misses 14 19 +5
Partials 4 4
Continue to review full report at Codecov.
|
3 similar comments
Codecov Report
@@ Coverage Diff @@
## master #1559 +/- ##
==========================================
- Coverage 99.06% 98.79% -0.27%
==========================================
Files 39 39
Lines 1915 1915
==========================================
- Hits 1897 1892 -5
- Misses 14 19 +5
Partials 4 4
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #1559 +/- ##
==========================================
- Coverage 99.06% 98.79% -0.27%
==========================================
Files 39 39
Lines 1915 1915
==========================================
- Hits 1897 1892 -5
- Misses 14 19 +5
Partials 4 4
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #1559 +/- ##
==========================================
- Coverage 99.06% 98.79% -0.27%
==========================================
Files 39 39
Lines 1915 1915
==========================================
- Hits 1897 1892 -5
- Misses 14 19 +5
Partials 4 4
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #1559 +/- ##
==========================================
- Coverage 99.27% 99.05% -0.22%
==========================================
Files 39 39
Lines 1928 1913 -15
==========================================
- Hits 1914 1895 -19
- Misses 10 14 +4
Partials 4 4
Continue to review full report at Codecov.
|
sponomarev
force-pushed
the
fix/load-html-tests
branch
2 times, most recently
from
d193cb3
to
2609878
Compare
- it's not necessary to rely on timeout, server starts listening synchronously and returns control when it is ready - once the server is run, it's stopped after test passes - dry out http server setup - preserve router.RunTLS coverage with integration tests
sponomarev
force-pushed
the
fix/load-html-tests
branch
from
2609878
to
bf8b57b
Compare
|
@sponomarev thanks a lot, LGTM, and need @appleboy review. |
Digging into the test code base I've found out that some of the tests for `LoadHTML*` methods are not reliable and efficient. They use timeouts to be sure that goroutine with the server has started. And even more, in old implementation, the server started only once – all the new instances silently failed due to the occupied network port. Here is a short overview of the proposed changes: - it's not necessary to rely on timeouts, the server starts listening synchronously and returns control when it is ready - once the server is run, it's stopped after a test passes - dry out http server setup - magic with empty closure return is eliminated - preserve router.RunTLS coverage with integration tests
Digging into the test code base I've found out that some of the tests for
LoadHTML*methods are not reliable and efficient. They use timeouts to be sure that goroutine with the server has started. And even more, in old implementation, the server started only once – all the new instances silently failed due to the occupied network port.Here is a short overview of the proposed changes: