Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Feat/migration ipfs download #8064
Feat/migration ipfs download #8064
Changes from 1 commit
bddfa4d
0565e80
b838384
b5aae2c
6963d0c
5bdef8c
f65110a
806831d
df9a2ca
0994fba
8a7a4c3
de0eaba
f9da061
1e16ba1
18bdb52
55f239b
ac8aa16
890bd7e
e5254d2
163c003
9768016
3a0be4c
ee577fe
9727b4f
7d16a95
96bad6f
d40d0f1
8d1dbfa
03454b7
bd6646f
3a4d85e
40d412a
e37896d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe hide it behind a flag, then :) otherwise the only way to run it is by editing the source.
is there any way to eventually make this test work in a local way without extra setup? e.g. by setting up some IPFS/HTTP servers/nodes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done - hidden behind flag
For the HttpFetcher I created a dummy test server. I am not really sure there is a great way to do this for IPFS. Mocking interfaces seems a bit excessive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm just a bit worried that we have a test that will practically never be run. Mocking isn't great, but if running a real IPFS node for the test is too difficult, at least testing with a mock by default would be better than testing nothing by default. The flag could still be used to test against a real IPFS node instead of a mock.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another pattern is to use an environment variable and put the name of it in the Skip message. Inspired by this blog post https://peter.bourgon.org/blog/2021/04/02/dont-use-build-tags-for-integration-tests.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was handled by adding a
skipUnlessEpic
function like as is done here:https://github.com/ipfs/go-ipfs/blob/ef866a1400b3b2861e5e8b6cc9edc8633b890a0a/test/integration/addcat_test.go#L173?