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(http): refactor MockRoundTripper #229

Merged
merged 1 commit into from
May 11, 2023

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented May 11, 2023

This is a little bit better I think, fewer stacked callbacks, no functions with long argument lists, a bit more Go-like and one less goroutine+channel in the chain.

This is a doubly-stacked PR, sorry, it's in to rvagg/http-validation which is in to feat/http. I'll retarget if necessary as things get merged.

@codecov-commenter
Copy link

codecov-commenter commented May 11, 2023

Codecov Report

Merging #229 (44f26dc) into rvagg/http-validation (7d45634) will increase coverage by 0.68%.
The diff coverage is 82.77%.

Additional details and impacted files

Impacted file tree graph

@@                    Coverage Diff                    @@
##           rvagg/http-validation     #229      +/-   ##
=========================================================
+ Coverage                  70.97%   71.66%   +0.68%     
=========================================================
  Files                         65       66       +1     
  Lines                       5685     5865     +180     
=========================================================
+ Hits                        4035     4203     +168     
- Misses                      1428     1441      +13     
+ Partials                     222      221       -1     
Impacted Files Coverage Δ
pkg/internal/testutil/mockroundtripper.go 82.77% <82.77%> (ø)

... and 5 files with indirect coverage changes

@hannahhoward hannahhoward changed the base branch from rvagg/http-validation to feat/http May 11, 2023 20:07
@hannahhoward hannahhoward force-pushed the rvagg/refactor-mock-roundtripper branch from 44f26dc to 8c9549b Compare May 11, 2023 20:09
@hannahhoward
Copy link
Collaborator

It looks shorter and sweeter, I'm inclined to approve esp since it's just test code.

@hannahhoward hannahhoward merged commit bb09696 into feat/http May 11, 2023
16 checks passed
rvagg added a commit that referenced this pull request May 12, 2023
hannahhoward added a commit that referenced this pull request May 12, 2023
* First pass at adapting graphsync to http

* fix(http): gracefully handle selector vs path requests

* feat(http): extend graphsyncretriever so it can also do http

* feat(http): refactor http & graphsync specific pieces to "TransportProtocol" iface

* feat(prioritywaitqueue): add InitialPauseDone inspector

* feat(http): single peer http retrieval unit test

* fix(http): enable http everywhere gs & bs are

* chore(http): framework for suite of http unit tests

based on bitswap unit test framework

* feat(http): remove parallel-request flow, make all serial for now

* too-detailed http testing, will remove this in favour of graphsync
  style testing.

* fix(http): better testing framework

* fix(http): more test coverage, minor fixes

* fix(http): clean up time handling in tests

* HTTP CAR validation (#222)

* feat(verifiedcar): initial verifiedcar package

* feat(verifiedcar): verify http retrievals

* chore(verifiedcar): tests for basic error cases

* fix(verifiedcar): coverage of more cases, handle known edges properly

* fix(verifiedcar): remove extraneous go-routine (#226)

Co-authored-by: Rod Vagg <rod@vagg.org>

* fix(verifiedcar): address feedback

* fix(verifiedcar): fix flaky tests

---------

Co-authored-by: Hannah Howard <hannah@hannahhoward.net>

* fix(http): refactor MockRoundTripper (#229)

* Add HTTP integegration tests (#227)

* test: add itests for http

* test: add peer http server, minor refactors and fixes

* fix(itest): fix compile errors on rebase

---------

Co-authored-by: Rod Vagg <rod@vagg.org>
Co-authored-by: Hannah Howard <hannah@hannahhoward.net>
Co-authored-by: Kyle Huntsman <3432646+kylehuntsman@users.noreply.github.com>
@kylehuntsman kylehuntsman deleted the rvagg/refactor-mock-roundtripper branch May 25, 2023 01:25
@kylehuntsman kylehuntsman restored the rvagg/refactor-mock-roundtripper branch May 25, 2023 01:25
@kylehuntsman kylehuntsman deleted the rvagg/refactor-mock-roundtripper branch May 25, 2023 01:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants