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

VerifiedCar: remove extraneous go-routine #226

Merged
merged 2 commits into from
May 10, 2023

Conversation

hannahhoward
Copy link
Collaborator

Goals

I was going to leave feedback about an extraneous go routine in verified car but I wanted to see if it worked first. It certainly appears to.

@hannahhoward hannahhoward mentioned this pull request May 10, 2023
@codecov-commenter
Copy link

codecov-commenter commented May 10, 2023

Codecov Report

Merging #226 (47d0e78) into rvagg/http-validation (e614c83) will increase coverage by 0.27%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@                    Coverage Diff                    @@
##           rvagg/http-validation     #226      +/-   ##
=========================================================
+ Coverage                  71.08%   71.36%   +0.27%     
=========================================================
  Files                         65       65              
  Lines                       5713     5685      -28     
=========================================================
- Hits                        4061     4057       -4     
+ Misses                      1432     1416      -16     
+ Partials                     220      212       -8     
Impacted Files Coverage Δ
pkg/verifiedcar/verifiedcar.go 80.18% <100.00%> (+0.33%) ⬆️

... and 6 files with indirect coverage changes

@hannahhoward hannahhoward requested a review from rvagg May 10, 2023 22:55
Copy link
Member

@rvagg rvagg left a comment

Choose a reason for hiding this comment

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

love it! well spotted and nicely simplified

@rvagg rvagg merged commit fb52261 into rvagg/http-validation May 10, 2023
@rvagg rvagg deleted the rvagg/http-validation-no-go-routine branch May 10, 2023 23:32
hannahhoward added a commit that referenced this pull request May 11, 2023
* 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>
rvagg added a commit that referenced this pull request May 12, 2023
* 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>
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>
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.

3 participants