Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: don't use foo-bar.net in TestHTTPClientMakeHTTPDialer #174

Merged
merged 2 commits into from
Feb 4, 2021

Conversation

wetcod
Copy link
Contributor

@wetcod wetcod commented Feb 2, 2021

Description

CI fails because of test relied on connecting to the external site.
So cherry-pick tendermint/tendermint#5997

And increase the test_cover timeout


For contributor use:

  • Wrote tests
  • Updated CHANGELOG_PENDING.md
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Updated relevant documentation (docs/) and code comments
  • Re-reviewed Files changed in the Github PR explorer

This test relied on connecting to the external site `foo-bar.net`, and (predictably) the site went down and broke all of our CI runs. This changes it to use local HTTP servers instead.

(cherry picked from commit f54f80bf0d464ffec6facf98e447ee403bbf1e8c)
@wetcod wetcod self-assigned this Feb 2, 2021
@wetcod wetcod added the C: test Classification: Add test case label Feb 2, 2021
@wetcod wetcod requested review from tnasu, Kynea0b and torao February 2, 2021 06:28
@codecov
Copy link

codecov bot commented Feb 2, 2021

Codecov Report

Merging #174 (02645eb) into develop (a01efd2) will decrease coverage by 0.08%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop     #174      +/-   ##
===========================================
- Coverage    62.52%   62.43%   -0.09%     
===========================================
  Files          283      283              
  Lines        26475    26480       +5     
===========================================
- Hits         16554    16534      -20     
- Misses        8576     8598      +22     
- Partials      1345     1348       +3     
Impacted Files Coverage Δ
lite2/trust_options.go 18.18% <0.00%> (-15.16%) ⬇️
types/tx.go 86.95% <0.00%> (-4.35%) ⬇️
blockchain/v2/reactor.go 35.92% <0.00%> (-2.97%) ⬇️
abci/client/socket_client.go 48.19% <0.00%> (-1.36%) ⬇️
consensus/state.go 77.10% <0.00%> (-0.92%) ⬇️
blockchain/v0/pool.go 78.02% <0.00%> (-0.64%) ⬇️
consensus/reactor.go 79.63% <0.00%> (-0.12%) ⬇️
p2p/pex/pex_reactor.go 83.33% <0.00%> (+0.55%) ⬆️
consensus/replay.go 71.42% <0.00%> (+0.77%) ⬆️
consensus/ticker.go 95.83% <0.00%> (+4.16%) ⬆️

Copy link
Contributor

@torao torao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@Kynea0b Kynea0b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wetcod
Copy link
Contributor Author

wetcod commented Feb 4, 2021

What does this value Hi!\n mean ?

This is cherry-picked PR, so I don't know exactly what the original author meant.
But it looks like sample bytes.

@wetcod wetcod merged commit d417f1d into Finschia:develop Feb 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: test Classification: Add test case
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants