Skip to content
This repository has been archived by the owner on Feb 1, 2023. It is now read-only.

Feat/bitswap pr improvements #19

Merged
merged 3 commits into from
Oct 29, 2018
Merged

Conversation

hannahhoward
Copy link
Contributor

@hannahhoward hannahhoward commented Oct 29, 2018

Goals

Bring over useful additional testing from #8 as well as fix issue with Bitswap sessions not matching sessions interface.

Implementation

Copy two relevant commits from that PR, and also copy the full dup_blocks_test.go file

For discussion

Should the interface fix and tests be separated?

Fixes ipfs/kubo#5640

@ghost ghost assigned hannahhoward Oct 29, 2018
@ghost ghost added the status/in-progress In progress label Oct 29, 2018
@hannahhoward
Copy link
Contributor Author

Edited the dup_blocks_test to run as a benchmark -- cause that's what it is. It was failing even in @whyrusleeping PR for me on my machine, which isn't that surprising.

@whyrusleeping
Copy link
Member

@hannahhoward yeah, i had the sensetivity on those tests set to zero, so any duplicate blocks caused a failure

So that CI passes, and because it's not reliable as a test, and is more a benchmark to measure
performance, convert dup_block_test.go to a benchmark, which can be run using `go test -bench .`
@hannahhoward
Copy link
Contributor Author

@whyrusleeping That makes sense. Also though w/o code to actually mitigate the issue, it makes sense to think of this as a benchmark. Once such code exists, maybe we make it a test to prevent regressions.

@Stebalien Stebalien merged commit 6338b06 into master Oct 29, 2018
@ghost ghost removed the status/in-progress In progress label Oct 29, 2018
@Stebalien
Copy link
Member

(it passed the merge test...)

@Stebalien Stebalien deleted the feat/bitswap-pr-improvements branch November 2, 2018 21:16
Jorropo pushed a commit to Jorropo/go-libipfs that referenced this pull request Jan 26, 2023
…vements

Feat/bitswap pr improvements

This commit was moved from ipfs/go-bitswap@6338b06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Are Bitswap Sessions actually being used?
3 participants