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.

Sign up for GitHub

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

Jump to bottom

bitswap_test: make racy test less racy #4109

Merged
merged 2 commits into from
Aug 17, 2017
Merged

bitswap_test: make racy test less racy #4109

merged 2 commits into from
Aug 17, 2017

Conversation

Stebalien
Copy link
Member

fixes #4108

@Stebalien
Copy link
Member Author

(in an effort to make CI reliable...)

@Stebalien
Copy link
Member Author

As usual, the failures are unrelated... (99 failures of tests on CI, 99 failures of tests, take one down, pass it around, 9001 failures of tests on CI).

Kubuxu
Kubuxu approved these changes Jul 31, 2017
View reviewed changes
Copy link
Member

@Kubuxu Kubuxu left a comment

Choose a reason for hiding this comment

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

Very good change, it will take us a while to catch them all.

We should also object tests that depend on flat timing in future.

@Stebalien
Copy link
Member Author

We should also object tests that depend on flat timing in future.

YES! Also, a rule that all CI failures must be investigated/reported, even if they look spurious. Always having to re-run CI a few times to get a pass is time consuming (and dangerous).

@Stebalien
Copy link
Member Author

Speaking of which... the travis test failure was #4062.

@whyrusleeping
Copy link
Member

we should add a more generic waitFor(ctx, check) method so we can do things like this more frequently.

@Stebalien Stebalien self-assigned this Aug 3, 2017
@Stebalien Stebalien added the status/blocked Unable to be worked further until needs are met label Aug 3, 2017
@Stebalien
Copy link
Member Author

Need to:

  1. Copy the modifications made to the vendored testutils to the actual library.
  2. Unvendor testutils.
  3. Use WaitFor from the unvendored testutils.

@whyrusleeping
Copy link
Member

@Stebalien update here?

@Stebalien
Copy link
Member Author

No change. I'll do this now... (it's just really tedious).

@Stebalien
Copy link
Member Author

Blocked on #4091 apparently (I need to update go-libp2p which now pulls in go-libp2p-circuit).

@vyzo
Copy link
Contributor

vyzo commented Aug 16, 2017

Note that #4091 is still in circuit-relay-1.1.2, it will need to update to circuit-relay-1.1.3 when libp2p is updated.

@Stebalien Stebalien removed the status/blocked Unable to be worked further until needs are met label Aug 16, 2017
@Stebalien Stebalien force-pushed the fix/4108 branch 3 times, most recently from c93b52b to 8d5f3f4 Compare August 17, 2017 00:14
@Stebalien
extract update go-testutil
19c3bd8 
License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
@Stebalien
bitswap_test: make racy test less racy
67c572e 
fixes #4108

License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
Stebalien
Stebalien commented Aug 17, 2017
View reviewed changes
core/core_test.go
@@ -38,7 +38,7 @@ func TestInitialization(t *testing.T) {
for i, c := range good {
r := &repo.Mock{
C: *c,
D: testutil.ThreadSafeCloserMapDatastore(),
D: ds2.ThreadSafeCloserMapDatastore(),
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: I moved this function from the vendored testutil library to datastore2.

@whyrusleeping whyrusleeping merged commit b6eb085 into master Aug 17, 2017
@whyrusleeping whyrusleeping deleted the fix/4108 branch August 17, 2017 02:46
@Kubuxu
Copy link
Member

Kubuxu commented Aug 17, 2017

Thank you for this.

@Kubuxu
Copy link
Member

Kubuxu commented Aug 17, 2017

Thank you a lot for this fix.

hacdias pushed a commit to ipfs/boxo that referenced this pull request Jan 27, 2023
@whyrusleeping
Merge pull request ipfs/kubo#4109 from ipfs/fix/4108
5fcdbb9 
bitswap_test: make racy test less racy

This commit was moved from ipfs/kubo@b6eb085
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kubuxu Kubuxu Kubuxu approved these changes

@whyrusleeping whyrusleeping whyrusleeping approved these changes

Assignees

@Stebalien Stebalien

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Random test failure in bitswap
4 participants
@Stebalien @whyrusleeping @vyzo @Kubuxu