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

fix(gateway): switch to separate Listen/Serve #1314

Merged
merged 7 commits into from
Nov 9, 2022

Conversation

distractedm1nd
Copy link
Collaborator

Closes #1286

This is mainly a fix for test flakiness.

@distractedm1nd distractedm1nd added kind:fix Attached to bug-fixing PRs area:gateway Related to the node's gateway labels Nov 2, 2022
@distractedm1nd distractedm1nd self-assigned this Nov 2, 2022
api/gateway/server.go Outdated Show resolved Hide resolved
renaynay
renaynay previously approved these changes Nov 2, 2022
@distractedm1nd
Copy link
Collaborator Author

Also is a part of #1306

renaynay
renaynay previously approved these changes Nov 3, 2022
api/rpc_test.go Outdated Show resolved Hide resolved
@walldiss
Copy link
Member

walldiss commented Nov 3, 2022

Do we still consider moving our rpc test infra to httptest.Server in future?

api/rpc_test.go Show resolved Hide resolved
nodebuilder/testing.go Outdated Show resolved Hide resolved
@distractedm1nd
Copy link
Collaborator Author

Now closes #1322 . Deprecates #1286 .

@distractedm1nd distractedm1nd changed the title fix(gateway): switch to ListenAndServe and use GetFreePort instead of port 0 fix(gateway): switch to separate Listen/Serve Nov 9, 2022
Co-authored-by: rene <41963722+renaynay@users.noreply.github.com>
Wondertan
Wondertan previously approved these changes Nov 9, 2022
Copy link
Member

@Wondertan Wondertan left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov-commenter
Copy link

codecov-commenter commented Nov 9, 2022

Codecov Report

Merging #1314 (c80c216) into main (f5f5adb) will increase coverage by 0.34%.
The diff coverage is 59.09%.

@@            Coverage Diff             @@
##             main    #1314      +/-   ##
==========================================
+ Coverage   54.83%   55.17%   +0.34%     
==========================================
  Files         176      176              
  Lines       10552    10618      +66     
==========================================
+ Hits         5786     5859      +73     
+ Misses       4194     4174      -20     
- Partials      572      585      +13     
Impacted Files Coverage Δ
api/gateway/server.go 62.96% <50.00%> (-3.04%) ⬇️
api/rpc/server.go 63.04% <60.00%> (-6.41%) ⬇️
nodebuilder/testing.go 100.00% <100.00%> (ø)
header/core/listener.go 52.83% <0.00%> (-5.67%) ⬇️
nodebuilder/rpc/module.go 91.66% <0.00%> (-2.93%) ⬇️
share/test_helpers.go 71.42% <0.00%> (-1.55%) ⬇️
share/ipld/get.go 91.20% <0.00%> (-1.39%) ⬇️
nodebuilder/rpc/rpc.go 100.00% <0.00%> (ø)
nodebuilder/das/daser.go 100.00% <0.00%> (ø)
nodebuilder/das/mocks/api.go 26.66% <0.00%> (ø)
... and 15 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@distractedm1nd distractedm1nd merged commit f7026ff into celestiaorg:main Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:gateway Related to the node's gateway kind:fix Attached to bug-fixing PRs
Projects
No open projects
Status: Done
6 participants