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

Add HTTP integegration tests #227

Merged
merged 2 commits into from May 12, 2023
Merged

Add HTTP integegration tests #227

merged 2 commits into from May 12, 2023

Conversation

kylehuntsman
Copy link
Contributor

@kylehuntsman kylehuntsman commented May 11, 2023

Adds HTTP integration tests that test that Lassie can fetch a valid car over the HTTP protocol. Remote HTTP peers are generated with an http multiaddress and run an HTTP server on that address for serving valid cars.

Notable Changes

  • itest/testpeer/generator.go can now generate HTTP peers with an HTTP server that returns valid cars
  • itest/testpeer/mocknet.go has been updated to add HTTP peers
  • Added itest/testpeer/peerhttpserver.go to create separate http server instances for each peer, as well as manage start and teardown appropriately.

@codecov-commenter
Copy link

codecov-commenter commented May 11, 2023

Codecov Report

Merging #227 (5331ef7) into feat/http (6810ff2) will increase coverage by 0.62%.
The diff coverage is 58.52%.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##           feat/http     #227      +/-   ##
=============================================
+ Coverage      70.66%   71.29%   +0.62%     
=============================================
  Files             64       67       +3     
  Lines           5543     6072     +529     
=============================================
+ Hits            3917     4329     +412     
- Misses          1413     1500      +87     
- Partials         213      243      +30     
Impacted Files Coverage Δ
pkg/internal/itest/testpeer/generator.go 62.96% <54.33%> (-7.67%) ⬇️
pkg/internal/itest/testpeer/peerhttpserver.go 65.71% <65.71%> (ø)
pkg/internal/itest/mocknet/mocknet.go 75.75% <78.57%> (-1.77%) ⬇️

... and 6 files with indirect coverage changes

@kylehuntsman kylehuntsman requested review from rvagg and hannahhoward and removed request for rvagg May 11, 2023 00:29
Copy link
Collaborator

@hannahhoward hannahhoward left a comment

Choose a reason for hiding this comment

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

After fixing @rvagg 's notes about UnixFS setup (sorry, IPLD is weird), we should test pathing over HTTP too

pkg/internal/itest/mocknet/mocknet.go Outdated Show resolved Hide resolved
pkg/internal/itest/testpeer/generator.go Outdated Show resolved Hide resolved
pkg/internal/itest/testpeer/generator.go Outdated Show resolved Hide resolved
pkg/internal/itest/testpeer/generator.go Outdated Show resolved Hide resolved
pkg/internal/itest/testpeer/generator.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@hannahhoward hannahhoward left a comment

Choose a reason for hiding this comment

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

After fixing @rvagg 's notes about UnixFS setup (sorry, IPLD is weird), we should test pathing over HTTP too

@hannahhoward
Copy link
Collaborator

but pathed tests can be in a seperate PR, lower priority.

@kylehuntsman kylehuntsman marked this pull request as ready for review May 11, 2023 22:59
@@ -259,3 +335,107 @@ func StartAndWaitForReady(ctx context.Context, manager datatransfer.Manager) err
return err
}
}

func MockIpfsHandler(ctx context.Context, lsys linking.LinkSystem) func(http.ResponseWriter, *http.Request) {
Copy link
Member

Choose a reason for hiding this comment

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

seems like this can be moved in to peerhttpserver.go now?

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.

lgtm, I would like to see path tests in there though, they look like they're supported on the server side so it should just mean copying the graphsync ones

@hannahhoward hannahhoward merged commit 1e9f14b into feat/http May 12, 2023
2 of 16 checks passed
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 test/http-itests branch May 25, 2023 01:23
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

4 participants